Re: [libav-devel] [PATCH 1/7] configure: Drop unnecessary variables, shifts, and quotes in helper functions

[James Almer]

On 2/11/2018 4:09 PM, Diego Biurrun wrote:
> ---
>  configure | 21 +++--
>  1 file changed, 7 insertions(+), 14 deletions(-)
> 
> diff --git a/configure b/configure
> index ed930e6cd4..904131301d 100755
> --- a/configure
> +++ b/configure
> @@ -816,7 +816,7 @@ EOF
>  
>  check_insn(){
>  log check_insn "$@"
> -check_inline_asm ${1}_inline "\"$2\""
> +check_inline_asm ${1}_inline "$2"
>  echo "$2" | check_as && enable ${1}_external || disable ${1}_external
>  }
>  
> @@ -1097,11 +1097,9 @@ check_struct(){
>  check_builtin(){
>  log check_builtin "$@"
>  name=$1
> -headers=$2
> -builtin=$3
> -shift 3
> +shift
>  disable "$name"
> -check_code ld "$headers" "$builtin" "$@" && enable "$name"
> +check_code ld "$@" && enable "$name"
>  }
>  
>  check_compile_assert(){
> @@ -1118,25 +1116,20 @@ require(){
>  log require "$@"
>  name_version="$1"
>  name="${1%% *}"
> -headers="$2"
> -func="$3"
> -shift 3
> -check_lib $name "$headers" $func "$@" || die "ERROR: $name_version not 
> found"
> +shift
> +check_lib $name "$@" || die "ERROR: $name_version not found"
>  }
>  
>  require_header(){
>  log require_header "$@"
>  headers="$1"
> -shift
> -check_header "$headers" "$@" || die "ERROR: $headers not found"
> +check_header "$@" || die "ERROR: $headers not found"
>  }
>  
>  require_cpp_condition(){
>  log require_cpp_condition "$@"
> -header="$1"
>  condition="$2"
> -shift 2
> -check_cpp_condition "$header" "$condition" "$@" || die "ERROR: 
> $condition not satisfied"
> +check_cpp_condition "$@" || die "ERROR: $condition not satisfied"
>  }
>  
>  require_pkg_config(){

Does passing "$@" keep and propagate the arguments split in a way
check_code() and similar can properly identify and handle? Checks can
use more than one header after all.

Should be ok if that's the case.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] Add AV1 video decoding support through libaom

[Vittorio Giovara]

On Sun, Feb 11, 2018 at 11:50 AM, Diego Biurrun  wrote:

> On Fri, Feb 09, 2018 at 10:51:36AM +0100, Luca Barbato wrote:
> >  libavcodec/version.h   |   2 +-
> >
> > --- a/libavcodec/version.h
> > +++ b/libavcodec/version.h
> > @@ -28,7 +28,7 @@
> >
> >  #define LIBAVCODEC_VERSION_MAJOR 58
> > -#define LIBAVCODEC_VERSION_MINOR  8
> > +#define LIBAVCODEC_VERSION_MINOR  9
> >  #define LIBAVCODEC_VERSION_MICRO  0
>
> I guess we should decide what to do about bumping version when there
> is no change to installed headers, e.g. when a decoder is added without
> adding an AV_CODEC_ID. Anton has been saying that minor should not be
> bumped in those cases and I think he is right.
>

IMO nothing wrong with bumping the minor version, but if not at least micro
should be changed.
-- 
Vittorio
___
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!

[Mark Thompson]

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.

Thanks,

- Mark
___
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!

[James Almer]

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.

> + ctx, 0);
> +if (!unit->content_ref) {
> +av_freep(>content);
> +return AVERROR(ENOMEM);
> +}
> +
> +return 0;
> +}

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

Re: [libav-devel] [PATCH 6/8] vf_hwupload/hwmap: Support setting a fixed pool size

[Maxym Dmytrychenko]

On Mon, Feb 12, 2018 at 12:30 AM, wm4  wrote:

> On Sun, 11 Feb 2018 23:09:51 +
> Mark Thompson  wrote:
>
> > On 11/02/18 07:21, Song, Ruiling wrote:
> > > I have run some test against the patches. It works as described.
> >
> > Great - there is general agreement on that part now so I've applied that
> series.
> >
> > > Are you still against setting a default value within qsv_init_pool()?
> > > Users can easily override the default value through the interface you
> added.
> > > And if they did not set a value, the default value will be used.
> >
> > I don't mind the idea but I'm against the specificity of it, I guess.
> The single value will help in the isolated hwupload case, but it would be
> inconsistent with other APIs and doesn't suggest a sensible value to anyone
> who does need to set a fixed pool size.
> >
> > > If you are OK, I will re-submit the patch and add more comment on the
> magic value.
> > > If you are against, we can discuss it further for a better solution.
> >
> > Returning to suggestions made before, how would you feel about adding
> some way to determine from a device how big a frames context should be for
> it?
>

sounds like potential solution.


> > The simplest method to do that would be to add a new element to
> HWContextType containing a sensible value.  If it's zero, dynamic
> allocation is supported and noone needs to care any further.  If it's
> nonzero, then it becomes the default value for pool size used in the
> absence of any other choice.  It would be used in av_hwframe_ctx_init() if
> it's set on the type and initial_pool_size is zero, which would fix our
> hwupload case.  Probably it would want some way for the user to access it
> as well - new API for retrieving property values somehow, maybe?  (Not sure
> about that bit.)
> >
> > Would that work for you?  Anyone want to offer any criticism / other
> ideas which don't depend on semi-mythical rewrites of lavfi link
> negotiation?
>
> Still feels kind of arbitrary. What exactly determines the size the
> hwcontext will propose?
>

sort of HW specific value but with promise to be sufficient at most of the
cases.

Even if a constant value - HW has not much logic to find a better one
but pool will be sufficient.


> ___
> 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

Re: [libav-devel] [PATCH 6/8] vf_hwupload/hwmap: Support setting a fixed pool size

[Mark Thompson]

On 11/02/18 23:30, wm4 wrote:
> On Sun, 11 Feb 2018 23:09:51 +
> Mark Thompson  wrote:
> 
>> On 11/02/18 07:21, Song, Ruiling wrote:
>>> I have run some test against the patches. It works as described.  
>>
>> Great - there is general agreement on that part now so I've applied that 
>> series.
>>
>>> Are you still against setting a default value within qsv_init_pool()?
>>> Users can easily override the default value through the interface you added.
>>> And if they did not set a value, the default value will be used.  
>>
>> I don't mind the idea but I'm against the specificity of it, I guess.  The 
>> single value will help in the isolated hwupload case, but it would be 
>> inconsistent with other APIs and doesn't suggest a sensible value to anyone 
>> who does need to set a fixed pool size.
>>
>>> If you are OK, I will re-submit the patch and add more comment on the magic 
>>> value.
>>> If you are against, we can discuss it further for a better solution.  
>>
>> Returning to suggestions made before, how would you feel about adding some 
>> way to determine from a device how big a frames context should be for it?
>>
>> The simplest method to do that would be to add a new element to 
>> HWContextType containing a sensible value.  If it's zero, dynamic allocation 
>> is supported and noone needs to care any further.  If it's nonzero, then it 
>> becomes the default value for pool size used in the absence of any other 
>> choice.  It would be used in av_hwframe_ctx_init() if it's set on the type 
>> and initial_pool_size is zero, which would fix our hwupload case.  Probably 
>> it would want some way for the user to access it as well - new API for 
>> retrieving property values somehow, maybe?  (Not sure about that bit.)
>>
>> Would that work for you?  Anyone want to offer any criticism / other ideas 
>> which don't depend on semi-mythical rewrites of lavfi link negotiation?
> 
> Still feels kind of arbitrary. 

Yeah, but we need something arbitrary to get around this problem.

> What exactly determines the size the
> hwcontext will propose?

It would be something like the largest number of frames that the worst consumer 
of that frame type might need in some nebulous "typical" case, plus the number 
that a non-decoder producer might need (probably two).

But, more importantly, the arbitrary value can be set for each API 
independently...

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

Re: [libav-devel] [PATCH 6/8] vf_hwupload/hwmap: Support setting a fixed pool size

[wm4]

On Sun, 11 Feb 2018 23:09:51 +
Mark Thompson  wrote:

> On 11/02/18 07:21, Song, Ruiling wrote:
> > I have run some test against the patches. It works as described.  
> 
> Great - there is general agreement on that part now so I've applied that 
> series.
> 
> > Are you still against setting a default value within qsv_init_pool()?
> > Users can easily override the default value through the interface you added.
> > And if they did not set a value, the default value will be used.  
> 
> I don't mind the idea but I'm against the specificity of it, I guess.  The 
> single value will help in the isolated hwupload case, but it would be 
> inconsistent with other APIs and doesn't suggest a sensible value to anyone 
> who does need to set a fixed pool size.
> 
> > If you are OK, I will re-submit the patch and add more comment on the magic 
> > value.
> > If you are against, we can discuss it further for a better solution.  
> 
> Returning to suggestions made before, how would you feel about adding some 
> way to determine from a device how big a frames context should be for it?
> 
> The simplest method to do that would be to add a new element to HWContextType 
> containing a sensible value.  If it's zero, dynamic allocation is supported 
> and noone needs to care any further.  If it's nonzero, then it becomes the 
> default value for pool size used in the absence of any other choice.  It 
> would be used in av_hwframe_ctx_init() if it's set on the type and 
> initial_pool_size is zero, which would fix our hwupload case.  Probably it 
> would want some way for the user to access it as well - new API for 
> retrieving property values somehow, maybe?  (Not sure about that bit.)
> 
> Would that work for you?  Anyone want to offer any criticism / other ideas 
> which don't depend on semi-mythical rewrites of lavfi link negotiation?

Still feels kind of arbitrary. What exactly determines the size the
hwcontext will propose?
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH 6/8] vf_hwupload/hwmap: Support setting a fixed pool size

[Mark Thompson]

On 11/02/18 07:21, Song, Ruiling wrote:
> I have run some test against the patches. It works as described.

Great - there is general agreement on that part now so I've applied that series.

> Are you still against setting a default value within qsv_init_pool()?
> Users can easily override the default value through the interface you added.
> And if they did not set a value, the default value will be used.

I don't mind the idea but I'm against the specificity of it, I guess.  The 
single value will help in the isolated hwupload case, but it would be 
inconsistent with other APIs and doesn't suggest a sensible value to anyone who 
does need to set a fixed pool size.

> If you are OK, I will re-submit the patch and add more comment on the magic 
> value.
> If you are against, we can discuss it further for a better solution.

Returning to suggestions made before, how would you feel about adding some way 
to determine from a device how big a frames context should be for it?

The simplest method to do that would be to add a new element to HWContextType 
containing a sensible value.  If it's zero, dynamic allocation is supported and 
noone needs to care any further.  If it's nonzero, then it becomes the default 
value for pool size used in the absence of any other choice.  It would be used 
in av_hwframe_ctx_init() if it's set on the type and initial_pool_size is zero, 
which would fix our hwupload case.  Probably it would want some way for the 
user to access it as well - new API for retrieving property values somehow, 
maybe?  (Not sure about that bit.)

Would that work for you?  Anyone want to offer any criticism / other ideas 
which don't depend on semi-mythical rewrites of lavfi link negotiation?

Thanks,

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

Re: [libav-devel] the big configure cleanup (act VI)

[Luca Barbato]

On 11/02/2018 20:09, Diego Biurrun wrote:

Today's installment of the series brings new helper functions to refactor
recurring patterns.

I settled for the naming scheme of using a test_ prefix for the helper
functions that do not set result variables and a check_ prefix for those
that do. Alternative/better names are welcome.



Looks fine, assuming it survives fate.

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!

[Mark Thompson]

On 11/02/18 20:13, wm4 wrote:
> On Sun, 11 Feb 2018 18:14:34 +
> Mark Thompson  wrote:
> 
>> ---
>>  libavcodec/cbs.c   | 168 +---
>>  libavcodec/cbs.h   |  51 -
>>  libavcodec/cbs_h264.h  |   4 +
>>  libavcodec/cbs_h2645.c | 341 
>> +++--
>>  libavcodec/cbs_h265.h  |   2 +
>>  libavcodec/cbs_internal.h  |   3 -
>>  libavcodec/cbs_mpeg2.c |  91 -
>>  libavcodec/cbs_mpeg2.h |   5 +
>>  libavcodec/cbs_mpeg2_syntax_template.c |   5 +-
>>  libavcodec/h264_metadata_bsf.c |   6 +-
>>  libavcodec/h265_metadata_bsf.c |   2 +-
>>  libavcodec/mpeg2_metadata_bsf.c|   3 +-
>>  libavcodec/vaapi_encode_h264.c |   2 +-
>>  libavcodec/vaapi_encode_h265.c |   2 +-
>>  libavcodec/vaapi_encode_mpeg2.c|   2 +-
>>  15 files changed, 408 insertions(+), 279 deletions(-)
>>
> 
> What does it do, and does it help with anything? I'm find it a bit
> suspicious, because our refcount stuff has a lot of overhead (multiple
> mallocs...).

The immediate intent is to make it easier to get alloc/free right - all 
subelements use the buffer API so that it's clear how to free them.  That is 
particularly helpful for stream-editing cases when you add or remove elements; 
VAAPI mostly doesn't mind because it points to the same fixed structures 
repeatedly.

It also elides some redundant copies, which are a much greater overhead than 
the additional small reference allocations.  The packet -> fragment copy is 
removed here if the input packet is refcounted.  The fragment -> unit copy can 
be elided for MPEG-2, but I haven't yet done that because the details are 
slightly subtle.  H.26[45] is trickier because it also undoes the emulation 
prevention, but there is a redundant copy NAL RBSP -> unit which should be 
removable with suitable changes to ff_h2645_packet_split() as well.

It also lets me move the slice_group_id element out of the PPS structure - 
having a >100k array there that was ~never used was kindof stupid.

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

[libav-devel] [PATCH] ivf: Support VP9 and AV1 as well

[Luca Barbato]

---

 libavformat/ivfenc.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/libavformat/ivfenc.c b/libavformat/ivfenc.c
index 1e57106993..48186a8907 100644
--- a/libavformat/ivfenc.c
+++ b/libavformat/ivfenc.c
@@ -18,6 +18,7 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */
 #include "avformat.h"
+#include "riff.h"
 #include "libavutil/intreadwrite.h"

 static int ivf_write_header(AVFormatContext *s)
@@ -30,14 +31,17 @@ static int ivf_write_header(AVFormatContext *s)
 return AVERROR(EINVAL);
 }
 par = s->streams[0]->codecpar;
-if (par->codec_type != AVMEDIA_TYPE_VIDEO || par->codec_id != 
AV_CODEC_ID_VP8) {
-av_log(s, AV_LOG_ERROR, "Currently only VP8 is supported!\n");
+if (par->codec_type != AVMEDIA_TYPE_VIDEO ||
+!(par->codec_id == AV_CODEC_ID_AV1 ||
+  par->codec_id == AV_CODEC_ID_VP8 ||
+  par->codec_id == AV_CODEC_ID_VP9)) {
+av_log(s, AV_LOG_ERROR, "Currently only AV1, VP8 and VP9 are 
supported!\n");
 return AVERROR(EINVAL);
 }
 avio_write(pb, "DKIF", 4);
 avio_wl16(pb, 0); // version
 avio_wl16(pb, 32); // header length
-avio_wl32(pb, par->codec_tag ? par->codec_tag : AV_RL32("VP80"));
+avio_wl32(pb, par->codec_tag ? par->codec_tag : 
ff_codec_get_tag(ff_codec_bmp_tags, par->codec_id));
 avio_wl16(pb, par->width);
 avio_wl16(pb, par->height);
 avio_wl32(pb, s->streams[0]->time_base.den);
--
2.12.2

___
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!

[wm4]

On Sun, 11 Feb 2018 18:14:34 +
Mark Thompson  wrote:

> ---
>  libavcodec/cbs.c   | 168 +---
>  libavcodec/cbs.h   |  51 -
>  libavcodec/cbs_h264.h  |   4 +
>  libavcodec/cbs_h2645.c | 341 
> +++--
>  libavcodec/cbs_h265.h  |   2 +
>  libavcodec/cbs_internal.h  |   3 -
>  libavcodec/cbs_mpeg2.c |  91 -
>  libavcodec/cbs_mpeg2.h |   5 +
>  libavcodec/cbs_mpeg2_syntax_template.c |   5 +-
>  libavcodec/h264_metadata_bsf.c |   6 +-
>  libavcodec/h265_metadata_bsf.c |   2 +-
>  libavcodec/mpeg2_metadata_bsf.c|   3 +-
>  libavcodec/vaapi_encode_h264.c |   2 +-
>  libavcodec/vaapi_encode_h265.c |   2 +-
>  libavcodec/vaapi_encode_mpeg2.c|   2 +-
>  15 files changed, 408 insertions(+), 279 deletions(-)
> 

What does it do, and does it help with anything? I'm find it a bit
suspicious, because our refcount stuff has a lot of overhead (multiple
mallocs...).
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] Add AV1 video decoding support through libaom

[wm4]

On Sun, 11 Feb 2018 18:44:44 +
Mark Thompson  wrote:

> On 10/02/18 16:29, Luca Barbato wrote:
> > On 10/02/2018 16:59, Diego Biurrun wrote:  
> >> Looks OK in general.
> >>
> >> On Fri, Feb 09, 2018 at 10:51:36AM +0100, Luca Barbato wrote:  
> >>> --- /dev/null
> >>> +++ b/libavcodec/libaom.c
> >>> @@ -0,0 +1,90 @@
> >>> +
> >>> +#define HIGH_DEPTH(fmt)   \
> >>> +    case AOM_IMG_FMT_I ## fmt ## 16: switch (depth) { \
> >>> +    case 10: return AV_PIX_FMT_YUV ## fmt ## P10; \
> >>> +    case 12: return AV_PIX_FMT_YUV ## fmt ## P12; \
> >>> +    default: return AV_PIX_FMT_NONE;  \  
> >>
> >> Move the switch statement to the next line for better readability please.  
> > 
> > Sure
> >   
> >>  
> >>> +enum AVPixelFormat ff_aom_imgfmt_to_pixfmt(aom_img_fmt_t img, int depth)
> >>> +{
> >>> +    switch (img) {
> >>> +    case AOM_IMG_FMT_RGB24: return AV_PIX_FMT_RGB24;
> >>> +    case AOM_IMG_FMT_RGB565:    return AV_PIX_FMT_RGB565BE;
> >>> +    case AOM_IMG_FMT_RGB555:    return AV_PIX_FMT_RGB555BE;
> >>> +    case AOM_IMG_FMT_UYVY:  return AV_PIX_FMT_UYVY422;
> >>> +    case AOM_IMG_FMT_YUY2:  return AV_PIX_FMT_YUYV422;
> >>> +    case AOM_IMG_FMT_YVYU:  return AV_PIX_FMT_YVYU422;
> >>> +    case AOM_IMG_FMT_BGR24: return AV_PIX_FMT_BGR24;
> >>> +    case AOM_IMG_FMT_ARGB:  return AV_PIX_FMT_ARGB;
> >>> +    case AOM_IMG_FMT_ARGB_LE:   return AV_PIX_FMT_BGRA;
> >>> +    case AOM_IMG_FMT_RGB565_LE: return AV_PIX_FMT_RGB565LE;
> >>> +    case AOM_IMG_FMT_RGB555_LE: return AV_PIX_FMT_RGB555LE;
> >>> +    case AOM_IMG_FMT_I420:  return AV_PIX_FMT_YUV420P;
> >>> +    case AOM_IMG_FMT_I422:  return AV_PIX_FMT_YUV422P;
> >>> +    case AOM_IMG_FMT_I444:  return AV_PIX_FMT_YUV444P;
> >>> +    case AOM_IMG_FMT_444A:  return AV_PIX_FMT_YUVA444P;
> >>> +    case AOM_IMG_FMT_I440:  return AV_PIX_FMT_YUV440P;  
> >>
> >> I'd break those lines.  
> > 
> > I can run uncrustify to break it, is that the outcome you'd expect?
> >   
> >>> +/*    case AOM_IMG_FMT_I42016:    return AV_PIX_FMT_YUV420P16;
> >>> +    case AOM_IMG_FMT_I42216:    return AV_PIX_FMT_YUV422P16;
> >>> +    case AOM_IMG_FMT_I44416:    return AV_PIX_FMT_YUV444P16; */  
> >>
> >> Why is this commented out?  
> > 
> > I should just remove it, thanks for reminding me.
> >   
> >>> +aom_img_fmt_t ff_aom_pixfmt_to_imgfmt(enum AVPixelFormat pix)
> >>> +{
> >>> +    switch (pix) {
> >>> +    case AV_PIX_FMT_RGB24:    return AOM_IMG_FMT_RGB24;
> >>> +    case AV_PIX_FMT_RGB565BE: return AOM_IMG_FMT_RGB565;
> >>> +    case AV_PIX_FMT_RGB555BE: return AOM_IMG_FMT_RGB555;
> >>> +    case AV_PIX_FMT_UYVY422:  return AOM_IMG_FMT_UYVY;
> >>> +    case AV_PIX_FMT_YUYV422:  return AOM_IMG_FMT_YUY2;
> >>> +    case AV_PIX_FMT_YVYU422:  return AOM_IMG_FMT_YVYU;
> >>> +    case AV_PIX_FMT_BGR24:    return AOM_IMG_FMT_BGR24;
> >>> +    case AV_PIX_FMT_ARGB: return AOM_IMG_FMT_ARGB;
> >>> +    case AV_PIX_FMT_BGRA: return AOM_IMG_FMT_ARGB_LE;
> >>> +    case AV_PIX_FMT_RGB565LE: return AOM_IMG_FMT_RGB565_LE;
> >>> +    case AV_PIX_FMT_RGB555LE: return AOM_IMG_FMT_RGB555_LE;
> >>> +    case AV_PIX_FMT_YUV420P:  return AOM_IMG_FMT_I420;
> >>> +    case AV_PIX_FMT_YUV422P:  return AOM_IMG_FMT_I422;
> >>> +    case AV_PIX_FMT_YUV444P:  return AOM_IMG_FMT_I444;
> >>> +    case AV_PIX_FMT_YUVA444P: return AOM_IMG_FMT_444A;
> >>> +    case AV_PIX_FMT_YUV440P:  return AOM_IMG_FMT_I440;
> >>> +    case AV_PIX_FMT_YUV420P10:    return AOM_IMG_FMT_I42016;
> >>> +    case AV_PIX_FMT_YUV422P10:    return AOM_IMG_FMT_I42216;
> >>> +    case AV_PIX_FMT_YUV444P10:    return AOM_IMG_FMT_I44416;
> >>> +    case AV_PIX_FMT_YUV420P12:    return AOM_IMG_FMT_I42016;
> >>> +    case AV_PIX_FMT_YUV422P12:    return AOM_IMG_FMT_I42216;
> >>> +    case AV_PIX_FMT_YUV444P12:    return AOM_IMG_FMT_I44416;  
> >>
> >> same  
> > 
> > Ok.  
> 
> If you feel like tearing down the bikeshed and building a different one: I 
> think this would be nicer as a single table with the two functions reading 
> it, rather than two functions which duplicate a lot of the information.

+1, this duplication is really not necessary.

Ignore my previous two empty replies, that's due to my mail client
being crap.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] Add AV1 video decoding support through libaom

[wm4]

On Sun, 11 Feb 2018 18:44:44 +
Mark Thompson  wrote:

> On 10/02/18 16:29, Luca Barbato wrote:
> > On 10/02/2018 16:59, Diego Biurrun wrote:  
> >> Looks OK in general.
> >>
> >> On Fri, Feb 09, 2018 at 10:51:36AM +0100, Luca Barbato wrote:  
> >>> --- /dev/null
> >>> +++ b/libavcodec/libaom.c
> >>> @@ -0,0 +1,90 @@
> >>> +
> >>> +#define HIGH_DEPTH(fmt)   \
> >>> +    case AOM_IMG_FMT_I ## fmt ## 16: switch (depth) { \
> >>> +    case 10: return AV_PIX_FMT_YUV ## fmt ## P10; \
> >>> +    case 12: return AV_PIX_FMT_YUV ## fmt ## P12; \
> >>> +    default: return AV_PIX_FMT_NONE;  \  
> >>
> >> Move the switch statement to the next line for better readability please.  
> > 
> > Sure
> >   
> >>  
> >>> +enum AVPixelFormat ff_aom_imgfmt_to_pixfmt(aom_img_fmt_t img, int depth)
> >>> +{
> >>> +    switch (img) {
> >>> +    case AOM_IMG_FMT_RGB24: return AV_PIX_FMT_RGB24;
> >>> +    case AOM_IMG_FMT_RGB565:    return AV_PIX_FMT_RGB565BE;
> >>> +    case AOM_IMG_FMT_RGB555:    return AV_PIX_FMT_RGB555BE;
> >>> +    case AOM_IMG_FMT_UYVY:  return AV_PIX_FMT_UYVY422;
> >>> +    case AOM_IMG_FMT_YUY2:  return AV_PIX_FMT_YUYV422;
> >>> +    case AOM_IMG_FMT_YVYU:  return AV_PIX_FMT_YVYU422;
> >>> +    case AOM_IMG_FMT_BGR24: return AV_PIX_FMT_BGR24;
> >>> +    case AOM_IMG_FMT_ARGB:  return AV_PIX_FMT_ARGB;
> >>> +    case AOM_IMG_FMT_ARGB_LE:   return AV_PIX_FMT_BGRA;
> >>> +    case AOM_IMG_FMT_RGB565_LE: return AV_PIX_FMT_RGB565LE;
> >>> +    case AOM_IMG_FMT_RGB555_LE: return AV_PIX_FMT_RGB555LE;
> >>> +    case AOM_IMG_FMT_I420:  return AV_PIX_FMT_YUV420P;
> >>> +    case AOM_IMG_FMT_I422:  return AV_PIX_FMT_YUV422P;
> >>> +    case AOM_IMG_FMT_I444:  return AV_PIX_FMT_YUV444P;
> >>> +    case AOM_IMG_FMT_444A:  return AV_PIX_FMT_YUVA444P;
> >>> +    case AOM_IMG_FMT_I440:  return AV_PIX_FMT_YUV440P;  
> >>
> >> I'd break those lines.  
> > 
> > I can run uncrustify to break it, is that the outcome you'd expect?
> >   
> >>> +/*    case AOM_IMG_FMT_I42016:    return AV_PIX_FMT_YUV420P16;
> >>> +    case AOM_IMG_FMT_I42216:    return AV_PIX_FMT_YUV422P16;
> >>> +    case AOM_IMG_FMT_I44416:    return AV_PIX_FMT_YUV444P16; */  
> >>
> >> Why is this commented out?  
> > 
> > I should just remove it, thanks for reminding me.
> >   
> >>> +aom_img_fmt_t ff_aom_pixfmt_to_imgfmt(enum AVPixelFormat pix)
> >>> +{
> >>> +    switch (pix) {
> >>> +    case AV_PIX_FMT_RGB24:    return AOM_IMG_FMT_RGB24;
> >>> +    case AV_PIX_FMT_RGB565BE: return AOM_IMG_FMT_RGB565;
> >>> +    case AV_PIX_FMT_RGB555BE: return AOM_IMG_FMT_RGB555;
> >>> +    case AV_PIX_FMT_UYVY422:  return AOM_IMG_FMT_UYVY;
> >>> +    case AV_PIX_FMT_YUYV422:  return AOM_IMG_FMT_YUY2;
> >>> +    case AV_PIX_FMT_YVYU422:  return AOM_IMG_FMT_YVYU;
> >>> +    case AV_PIX_FMT_BGR24:    return AOM_IMG_FMT_BGR24;
> >>> +    case AV_PIX_FMT_ARGB: return AOM_IMG_FMT_ARGB;
> >>> +    case AV_PIX_FMT_BGRA: return AOM_IMG_FMT_ARGB_LE;
> >>> +    case AV_PIX_FMT_RGB565LE: return AOM_IMG_FMT_RGB565_LE;
> >>> +    case AV_PIX_FMT_RGB555LE: return AOM_IMG_FMT_RGB555_LE;
> >>> +    case AV_PIX_FMT_YUV420P:  return AOM_IMG_FMT_I420;
> >>> +    case AV_PIX_FMT_YUV422P:  return AOM_IMG_FMT_I422;
> >>> +    case AV_PIX_FMT_YUV444P:  return AOM_IMG_FMT_I444;
> >>> +    case AV_PIX_FMT_YUVA444P: return AOM_IMG_FMT_444A;
> >>> +    case AV_PIX_FMT_YUV440P:  return AOM_IMG_FMT_I440;
> >>> +    case AV_PIX_FMT_YUV420P10:    return AOM_IMG_FMT_I42016;
> >>> +    case AV_PIX_FMT_YUV422P10:    return AOM_IMG_FMT_I42216;
> >>> +    case AV_PIX_FMT_YUV444P10:    return AOM_IMG_FMT_I44416;
> >>> +    case AV_PIX_FMT_YUV420P12:    return AOM_IMG_FMT_I42016;
> >>> +    case AV_PIX_FMT_YUV422P12:    return AOM_IMG_FMT_I42216;
> >>> +    case AV_PIX_FMT_YUV444P12:    return AOM_IMG_FMT_I44416;  
> >>
> >> same  
> > 
> > Ok.  
> 
> If you feel like tearing down the bikeshed and building a different one: I 
> think this would be nicer as a single table with the two functions reading 
> it, rather than two functions which duplicate a lot of the information.


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

Re: [libav-devel] [PATCH] Add AV1 video decoding support through libaom

[wm4]

On Sun, 11 Feb 2018 18:44:44 +
Mark Thompson  wrote:

> On 10/02/18 16:29, Luca Barbato wrote:
> > On 10/02/2018 16:59, Diego Biurrun wrote:  
> >> Looks OK in general.
> >>
> >> On Fri, Feb 09, 2018 at 10:51:36AM +0100, Luca Barbato wrote:  
> >>> --- /dev/null
> >>> +++ b/libavcodec/libaom.c
> >>> @@ -0,0 +1,90 @@
> >>> +
> >>> +#define HIGH_DEPTH(fmt)   \
> >>> +    case AOM_IMG_FMT_I ## fmt ## 16: switch (depth) { \
> >>> +    case 10: return AV_PIX_FMT_YUV ## fmt ## P10; \
> >>> +    case 12: return AV_PIX_FMT_YUV ## fmt ## P12; \
> >>> +    default: return AV_PIX_FMT_NONE;  \  
> >>
> >> Move the switch statement to the next line for better readability please.  
> > 
> > Sure
> >   
> >>  
> >>> +enum AVPixelFormat ff_aom_imgfmt_to_pixfmt(aom_img_fmt_t img, int depth)
> >>> +{
> >>> +    switch (img) {
> >>> +    case AOM_IMG_FMT_RGB24: return AV_PIX_FMT_RGB24;
> >>> +    case AOM_IMG_FMT_RGB565:    return AV_PIX_FMT_RGB565BE;
> >>> +    case AOM_IMG_FMT_RGB555:    return AV_PIX_FMT_RGB555BE;
> >>> +    case AOM_IMG_FMT_UYVY:  return AV_PIX_FMT_UYVY422;
> >>> +    case AOM_IMG_FMT_YUY2:  return AV_PIX_FMT_YUYV422;
> >>> +    case AOM_IMG_FMT_YVYU:  return AV_PIX_FMT_YVYU422;
> >>> +    case AOM_IMG_FMT_BGR24: return AV_PIX_FMT_BGR24;
> >>> +    case AOM_IMG_FMT_ARGB:  return AV_PIX_FMT_ARGB;
> >>> +    case AOM_IMG_FMT_ARGB_LE:   return AV_PIX_FMT_BGRA;
> >>> +    case AOM_IMG_FMT_RGB565_LE: return AV_PIX_FMT_RGB565LE;
> >>> +    case AOM_IMG_FMT_RGB555_LE: return AV_PIX_FMT_RGB555LE;
> >>> +    case AOM_IMG_FMT_I420:  return AV_PIX_FMT_YUV420P;
> >>> +    case AOM_IMG_FMT_I422:  return AV_PIX_FMT_YUV422P;
> >>> +    case AOM_IMG_FMT_I444:  return AV_PIX_FMT_YUV444P;
> >>> +    case AOM_IMG_FMT_444A:  return AV_PIX_FMT_YUVA444P;
> >>> +    case AOM_IMG_FMT_I440:  return AV_PIX_FMT_YUV440P;  
> >>
> >> I'd break those lines.  
> > 
> > I can run uncrustify to break it, is that the outcome you'd expect?
> >   
> >>> +/*    case AOM_IMG_FMT_I42016:    return AV_PIX_FMT_YUV420P16;
> >>> +    case AOM_IMG_FMT_I42216:    return AV_PIX_FMT_YUV422P16;
> >>> +    case AOM_IMG_FMT_I44416:    return AV_PIX_FMT_YUV444P16; */  
> >>
> >> Why is this commented out?  
> > 
> > I should just remove it, thanks for reminding me.
> >   
> >>> +aom_img_fmt_t ff_aom_pixfmt_to_imgfmt(enum AVPixelFormat pix)
> >>> +{
> >>> +    switch (pix) {
> >>> +    case AV_PIX_FMT_RGB24:    return AOM_IMG_FMT_RGB24;
> >>> +    case AV_PIX_FMT_RGB565BE: return AOM_IMG_FMT_RGB565;
> >>> +    case AV_PIX_FMT_RGB555BE: return AOM_IMG_FMT_RGB555;
> >>> +    case AV_PIX_FMT_UYVY422:  return AOM_IMG_FMT_UYVY;
> >>> +    case AV_PIX_FMT_YUYV422:  return AOM_IMG_FMT_YUY2;
> >>> +    case AV_PIX_FMT_YVYU422:  return AOM_IMG_FMT_YVYU;
> >>> +    case AV_PIX_FMT_BGR24:    return AOM_IMG_FMT_BGR24;
> >>> +    case AV_PIX_FMT_ARGB: return AOM_IMG_FMT_ARGB;
> >>> +    case AV_PIX_FMT_BGRA: return AOM_IMG_FMT_ARGB_LE;
> >>> +    case AV_PIX_FMT_RGB565LE: return AOM_IMG_FMT_RGB565_LE;
> >>> +    case AV_PIX_FMT_RGB555LE: return AOM_IMG_FMT_RGB555_LE;
> >>> +    case AV_PIX_FMT_YUV420P:  return AOM_IMG_FMT_I420;
> >>> +    case AV_PIX_FMT_YUV422P:  return AOM_IMG_FMT_I422;
> >>> +    case AV_PIX_FMT_YUV444P:  return AOM_IMG_FMT_I444;
> >>> +    case AV_PIX_FMT_YUVA444P: return AOM_IMG_FMT_444A;
> >>> +    case AV_PIX_FMT_YUV440P:  return AOM_IMG_FMT_I440;
> >>> +    case AV_PIX_FMT_YUV420P10:    return AOM_IMG_FMT_I42016;
> >>> +    case AV_PIX_FMT_YUV422P10:    return AOM_IMG_FMT_I42216;
> >>> +    case AV_PIX_FMT_YUV444P10:    return AOM_IMG_FMT_I44416;
> >>> +    case AV_PIX_FMT_YUV420P12:    return AOM_IMG_FMT_I42016;
> >>> +    case AV_PIX_FMT_YUV422P12:    return AOM_IMG_FMT_I42216;
> >>> +    case AV_PIX_FMT_YUV444P12:    return AOM_IMG_FMT_I44416;  
> >>
> >> same  
> > 
> > Ok.  
> 
> If you feel like tearing down the bikeshed and building a different one: I 
> think this would be nicer as a single table with the two functions reading 
> it, rather than two functions which duplicate a lot of the information.

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

[libav-devel] [PATCH 7/7] configure: Add check_as() helper function to simplify some expressions

[Diego Biurrun]

---
 configure | 37 ++---
 1 file changed, 18 insertions(+), 19 deletions(-)

diff --git a/configure b/configure
index 0582be0153..7ee19b0aae 100755
--- a/configure
+++ b/configure
@@ -811,6 +811,17 @@ check_cmd(){
 test_cmd $@ && enable $cmd
 }
 
+check_as(){
+log check_as "$@"
+name=$1
+code=$2
+shift 2
+disable $name
+test_as $@ 

[libav-devel] [PATCH 4/7] configure: Add check_cpp_condition() helper function to simplify some expressions

[Diego Biurrun]

---
 configure | 43 +++
 1 file changed, 27 insertions(+), 16 deletions(-)

diff --git a/configure b/configure
index cbac3d4210..3239461896 100755
--- a/configure
+++ b/configure
@@ -998,6 +998,14 @@ test_cpp_condition(){
 EOF
 }
 
+check_cpp_condition(){
+log check_cpp_condition "$@"
+name=$1
+shift 1
+disable $name
+test_cpp_condition "$@" && enable $name
+}
+
 test_cflags_cpp(){
 log test_cflags_cpp "$@"
 flags=$1
@@ -1197,6 +1205,14 @@ test_host_cpp_condition(){
 EOF
 }
 
+check_host_cpp_condition(){
+log check_host_cpp_condition "$@"
+name=$1
+shift 1
+disable $name
+test_host_cpp_condition "$@" && enable $name
+}
+
 cp_if_changed(){
 cmp -s "$1" "$2" && { test "$quiet" != "yes" && echo "$2 is unchanged"; } 
&& return
 mkdir -p "$(dirname $2)"
@@ -4223,14 +4239,14 @@ elif enabled alpha; then
 
 elif enabled arm; then
 
-enabled msvc && test_cpp_condition stddef.h "defined _M_ARMT" && enable 
thumb
+enabled msvc && check_cpp_condition thumb stddef.h "defined _M_ARMT"
 test_cpp_condition stddef.h "defined __thumb__" && enable_weak thumb
 enabled thumb && check_cflags -mthumb || check_cflags -marm
 
-if test_cpp_condition stddef.h "defined __ARM_PCS_VFP"; then
-enable vfp_args
-elif test_cpp_condition stddef.h "defined _M_ARM_FP && _M_ARM_FP >= 30"; 
then
-enable vfp_args
+if check_cpp_condition vfp_args stddef.h "defined __ARM_PCS_VFP"; then
+:
+elif check_cpp_condition vfp_args stddef.h "defined _M_ARM_FP && _M_ARM_FP 
>= 30"; then
+:
 elif ! test_cpp_condition stddef.h "defined __ARM_PCS || defined 
__SOFTFP__" && [ $target_os != darwin ]; then
 case "${cross_prefix:-$cc}" in
 *hardfloat*) enable vfp_args;   fpabi=vfp ;;
@@ -4325,7 +4341,7 @@ elif enabled ppc; then
 fi
 
 if enabled power8; then
-test_cpp_condition "altivec.h" "defined(_ARCH_PWR8)" || disable power8
+check_cpp_condition power8 altivec.h "defined(_ARCH_PWR8)"
 fi
 
 elif enabled x86; then
@@ -4592,10 +4608,7 @@ done
 
 # these are off by default, so fail if requested and not available
 
-enabled amf &&
-test_cpp_condition "AMF/core/Version.h" \
-"(AMF_VERSION_MAJOR << 48 | AMF_VERSION_MINOR << 32 | 
AMF_VERSION_RELEASE << 16 | AMF_VERSION_BUILD_NUM) >= 0x0001000400040001" ||
-disable amf
+enabled amf   && check_cpp_condition amf AMF/core/Version.h 
"(AMF_VERSION_MAJOR << 48 | AMF_VERSION_MINOR << 32 | AMF_VERSION_RELEASE << 16 
| AMF_VERSION_BUILD_NUM) >= 0x0001000400040001"
 enabled avisynth  && require_header avisynth/avisynth_c.h
 enabled avxsynth  && require_header avxsynth/avxsynth_c.h
 enabled cuda  && require cuda cuda.h cuInit -lcuda
@@ -4650,8 +4663,7 @@ enabled libwavpack&& require libwavpack 
wavpack/wavpack.h WavpackOpenFil
 enabled libwebp   && require_pkg_config libwebp libwebp webp/encode.h 
WebPGetEncoderVersion
 enabled libx264   && require_pkg_config libx264 x264 "stdint.h x264.h" 
x264_encoder_encode &&
  require_cpp_condition x264.h "X264_BUILD >= 118" 
&&
- { test_cpp_condition x264.h "X264_MPEG2" &&
-   enable libx262; }
+ check_cpp_condition libx262 x264.h "X264_MPEG2"
 enabled libx265   && require_pkg_config libx265 x265 x265.h 
x265_api_get &&
  require_cpp_condition x265.h "X265_BUILD >= 57"
 enabled libxavs   && require libxavs "stdint.h xavs.h" 
xavs_encoder_encode -lxavs
@@ -4691,7 +4703,7 @@ check_lib user32 "windows.h winuser.h" GetShellWindow 
-luser32
 check_lib vfw32 "windows.h vfw.h" capCreateCaptureWindow -lvfw32
 # check that WM_CAP_DRIVER_CONNECT is defined to the proper value
 # w32api 3.12 had it defined wrong
-test_cpp_condition vfw.h "WM_CAP_DRIVER_CONNECT > WM_USER" && enable 
vfwcap_defines
+check_cpp_condition vfwcap_defines vfw.h "WM_CAP_DRIVER_CONNECT > WM_USER"
 
 # check for ioctl_meteor.h, ioctl_bt848.h and alternatives
 check_header "dev/bktr/ioctl_meteor.h dev/bktr/ioctl_bt848.h"  
 ||
@@ -4756,8 +4768,7 @@ enabled vaapi &&
 enable vaapi_1
 
 enabled vdpau &&
-test_cpp_condition vdpau/vdpau.h "defined 
VDP_DECODER_PROFILE_MPEG4_PART2_ASP" ||
-disable vdpau
+check_cpp_condition vdpau vdpau/vdpau.h "defined 
VDP_DECODER_PROFILE_MPEG4_PART2_ASP"
 
 enabled vdpau &&
 check_lib vdpau_x11 "vdpau/vdpau.h vdpau/vdpau_x11.h" 
vdp_device_create_x11 -lvdpau -lX11
@@ -4965,7 +4976,7 @@ elif enabled_any msvc icl; then
 disable inline_asm
 fi
 # msvcrt10 x64 incorrectly enables log2, only msvcrt12 (MSVC 2013) onwards 
actually has log2.
-test_cpp_condition crtversion.h "_VC_CRT_MAJOR_VERSION >= 12" || disable 
log2
+check_cpp_condition log2 crtversion.h "_VC_CRT_MAJOR_VERSION >= 12"
 # 

[libav-devel] [PATCH 3/7] configure: Add check_cmd() helper function to simplify some expressions

[Diego Biurrun]

---
 configure | 14 +++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/configure b/configure
index fbb4ceb769..cbac3d4210 100755
--- a/configure
+++ b/configure
@@ -803,6 +803,14 @@ test_as(){
 test_cmd $as $CPPFLAGS $ASFLAGS "$@" $AS_C $(as_o $TMPO) $TMPS
 }
 
+check_cmd(){
+log check_cmd "$@"
+cmd=$1
+disabled $cmd && return
+disable $cmd
+test_cmd $@ && enable $cmd
+}
+
 check_inline_asm(){
 log check_inline_asm "$@"
 name="$1"
@@ -4671,9 +4679,6 @@ enabled openssl   && { { check_pkg_config openssl 
openssl openssl/ssl.h
 enabled avplay &&
 test_pkg_config sdl "sdl >= 1.2.1 sdl < 1.3.0" SDL_events.h SDL_PollEvent
 
-! disabled pod2man   && test_cmd pod2man --help && enable pod2man   || 
disable pod2man
-! disabled texi2html && test_cmd texi2html -version && enable texi2html || 
disable texi2html
-
 check_header linux/fb.h
 check_header linux/videodev2.h
 check_struct linux/videodev2.h "struct v4l2_frmivalenum" discrete
@@ -4998,6 +5003,9 @@ esac
 
 enabled asm || { arch=c; disable $ARCH_LIST $ARCH_EXT_LIST; }
 
+check_cmd pod2man --help
+check_cmd texi2html -version
+
 check_deps $CONFIG_LIST   \
$CONFIG_EXTRA  \
$HAVE_LIST \
-- 
2.11.0

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

[libav-devel] [PATCH 5/7] configure: Add check_cc/require_cc helper functions to simplify some expressions

[Diego Biurrun]

---
 configure | 71 ++-
 1 file changed, 34 insertions(+), 37 deletions(-)

diff --git a/configure b/configure
index 3239461896..0c71df8984 100755
--- a/configure
+++ b/configure
@@ -1129,6 +1129,14 @@ check_compile_assert(){
 test_code cc "$headers" "char c[2 * !!($condition) - 1]" "$@" && enable 
"$name"
 }
 
+check_cc(){
+log check_cc "$@"
+name=$1
+shift
+disable "$name"
+test_code cc "$@" && enable "$name"
+}
+
 require(){
 log require "$@"
 name_version="$1"
@@ -1137,6 +1145,13 @@ require(){
 check_lib $name "$@" || die "ERROR: $name_version not found"
 }
 
+require_cc(){
+log require_cc "$@"
+name="$1"
+shift
+test_code cc "$@" || die "ERROR: $name failed"
+}
+
 require_header(){
 log require_header "$@"
 headers="$1"
@@ -4163,18 +4178,12 @@ extern_prefix=${sym%%ff_extern*}
 ! disabled inline_asm && check_inline_asm inline_asm '"" ::'
 
 for restrict_keyword in restrict __restrict__ __restrict ""; do
-test_cc 

[libav-devel] [PATCH 1/7] configure: Drop unnecessary variables, shifts, and quotes in helper functions

[Diego Biurrun]

---
 configure | 21 +++--
 1 file changed, 7 insertions(+), 14 deletions(-)

diff --git a/configure b/configure
index ed930e6cd4..904131301d 100755
--- a/configure
+++ b/configure
@@ -816,7 +816,7 @@ EOF
 
 check_insn(){
 log check_insn "$@"
-check_inline_asm ${1}_inline "\"$2\""
+check_inline_asm ${1}_inline "$2"
 echo "$2" | check_as && enable ${1}_external || disable ${1}_external
 }
 
@@ -1097,11 +1097,9 @@ check_struct(){
 check_builtin(){
 log check_builtin "$@"
 name=$1
-headers=$2
-builtin=$3
-shift 3
+shift
 disable "$name"
-check_code ld "$headers" "$builtin" "$@" && enable "$name"
+check_code ld "$@" && enable "$name"
 }
 
 check_compile_assert(){
@@ -1118,25 +1116,20 @@ require(){
 log require "$@"
 name_version="$1"
 name="${1%% *}"
-headers="$2"
-func="$3"
-shift 3
-check_lib $name "$headers" $func "$@" || die "ERROR: $name_version not 
found"
+shift
+check_lib $name "$@" || die "ERROR: $name_version not found"
 }
 
 require_header(){
 log require_header "$@"
 headers="$1"
-shift
-check_header "$headers" "$@" || die "ERROR: $headers not found"
+check_header "$@" || die "ERROR: $headers not found"
 }
 
 require_cpp_condition(){
 log require_cpp_condition "$@"
-header="$1"
 condition="$2"
-shift 2
-check_cpp_condition "$header" "$condition" "$@" || die "ERROR: $condition 
not satisfied"
+check_cpp_condition "$@" || die "ERROR: $condition not satisfied"
 }
 
 require_pkg_config(){
-- 
2.11.0

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

[libav-devel] the big configure cleanup (act VI)

[Diego Biurrun]

Today's installment of the series brings new helper functions to refactor
recurring patterns.

I settled for the naming scheme of using a test_ prefix for the helper
functions that do not set result variables and a check_ prefix for those
that do. Alternative/better names are welcome.

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

[libav-devel] [PATCH 6/7] configure: Add check_ld() helper function to simplify some expressions

[Diego Biurrun]

---
 configure | 14 +++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/configure b/configure
index 0c71df8984..0582be0153 100755
--- a/configure
+++ b/configure
@@ -850,6 +850,14 @@ test_ld(){
 test_cmd $ld $LDFLAGS $flags $(ld_o $TMPE) $TMPO $libs $extralibs
 }
 
+check_ld(){
+log check_ld "$@"
+name=$1
+shift
+disable $name
+test_ld $@ && enable $name
+}
+
 print_include(){
 hdr=$1
 test "${hdr%.h}" = "${hdr}" &&
@@ -4258,8 +4266,8 @@ elif enabled arm; then
 :
 elif ! test_cpp_condition stddef.h "defined __ARM_PCS || defined 
__SOFTFP__" && [ $target_os != darwin ]; then
 case "${cross_prefix:-$cc}" in
-*hardfloat*) enable vfp_args;   fpabi=vfp ;;
-*) test_ld 

[libav-devel] [PATCH 2/7] configure: Use test_ prefix for helper functions that do not set variables

[Diego Biurrun]

---
 configure | 247 +++---
 1 file changed, 124 insertions(+), 123 deletions(-)

diff --git a/configure b/configure
index 904131301d..fbb4ceb769 100755
--- a/configure
+++ b/configure
@@ -758,7 +758,7 @@ add_compat(){
 map 'add_cppflags -D$v' "$@"
 }
 
-check_cmd(){
+test_cmd(){
 log "$@"
 "$@" >> $logfile 2>&1
 }
@@ -771,36 +771,36 @@ cc_e(){
 eval printf '%s\\n' $CC_E
 }
 
-check_cc(){
-log check_cc "$@"
+test_cc(){
+log test_cc "$@"
 cat > $TMPC
 log_file $TMPC
-check_cmd $cc $CPPFLAGS $CFLAGS "$@" $CC_C $(cc_o $TMPO) $TMPC
+test_cmd $cc $CPPFLAGS $CFLAGS "$@" $CC_C $(cc_o $TMPO) $TMPC
 }
 
-check_objcc(){
-log check_objcc "$@"
+test_objcc(){
+log test_objcc "$@"
 cat > $TMPC
 log_file $TMPC
-check_cmd $objcc $CPPFLAGS $CFLAGS $OBJCFLAGS "$@" $OBJCC_C $(cc_o $TMPO) 
$TMPC
+test_cmd $objcc $CPPFLAGS $CFLAGS $OBJCFLAGS "$@" $OBJCC_C $(cc_o $TMPO) 
$TMPC
 }
 
-check_cpp(){
-log check_cpp "$@"
+test_cpp(){
+log test_cpp "$@"
 cat > $TMPC
 log_file $TMPC
-check_cmd $cc $CPPFLAGS $CFLAGS "$@" $(cc_e $TMPO) $TMPC
+test_cmd $cc $CPPFLAGS $CFLAGS "$@" $(cc_e $TMPO) $TMPC
 }
 
 as_o(){
 eval printf '%s\\n' $AS_O
 }
 
-check_as(){
-log check_as "$@"
+test_as(){
+log test_as "$@"
 cat > $TMPS
 log_file $TMPS
-check_cmd $as $CPPFLAGS $ASFLAGS "$@" $AS_C $(as_o $TMPO) $TMPS
+test_cmd $as $CPPFLAGS $ASFLAGS "$@" $AS_C $(as_o $TMPO) $TMPS
 }
 
 check_inline_asm(){
@@ -809,7 +809,7 @@ check_inline_asm(){
 code="$2"
 shift 2
 disable $name
-check_cc "$@" < $TMPS
 log_file $TMPS
 shift 1
-check_cmd $x86asmexe $X86ASMFLAGS "$@" -o $TMPO $TMPS
+test_cmd $x86asmexe $X86ASMFLAGS "$@" -o $TMPO $TMPS
 }
 
 ld_o(){
 eval printf '%s\\n' $LD_O
 }
 
-check_ld(){
-log check_ld "$@"
+test_ld(){
+log test_ld "$@"
 flags=$(filter_out '-l*' "$@")
 libs=$(filter '-l*' "$@")
-check_cc $($cflags_filter $flags) || return
+test_cc $($cflags_filter $flags) || return
 flags=$($ldflags_filter $flags)
 libs=$($ldflags_filter $libs)
-check_cmd $ld $LDFLAGS $flags $(ld_o $TMPE) $TMPO $libs $extralibs
+test_cmd $ld $LDFLAGS $flags $(ld_o $TMPE) $TMPO $libs $extralibs
 }
 
 print_include(){
@@ -849,8 +849,8 @@ print_include(){
 echo "#include <$hdr>"
 }
 
-check_code(){
-log check_code "$@"
+test_code(){
+log test_code "$@"
 check=$1
 headers=$2
 code=$3
@@ -860,12 +860,12 @@ check_code(){
 print_include $hdr
 done
 echo "int main(void) { $code; return 0; }"
-} | check_$check "$@"
+} | test_$check "$@"
 }
 
 check_cppflags(){
 log check_cppflags "$@"
-check_cpp "$@" <;
 EOF
 }
@@ -873,7 +873,7 @@ EOF
 test_cflags(){
 log test_cflags "$@"
 set -- $($cflags_filter "$@")
-check_cc "$@" <
 float foo(float f, float g) { return $func($args); }
 int main(void){ return 0; }
@@ -974,15 +974,15 @@ check_func_headers(){
 echo "long check_$func(void) { return (long) $func; }"
 done
 echo "int main(void) { return 0; }"
-} | check_ld "$@" && enable $funcs && enable_sanitized $headers
+} | test_ld "$@" && enable $funcs && enable_sanitized $headers
 }
 
-check_cpp_condition(){
-log check_cpp_condition "$@"
+test_cpp_condition(){
+log test_cpp_condition "$@"
 header=$1
 condition=$2
 shift 2
-check_cpp "$@" <
 #if !($condition)
 #error "unsatisfied condition: $condition"
@@ -996,7 +996,7 @@ test_cflags_cpp(){
 condition=$2
 shift 2
 set -- $($cflags_filter "$flags")
-check_cpp "$@" <> $logfile 2>&1; }
+test_exec(){
+log test_exec "$@"
+test_ld "$@" && { enabled cross_compile || $TMPE >> $logfile 2>&1; }
 }
 
 check_exec_crash(){
@@ -1053,7 +1054,7 @@ check_exec_crash(){
 # can redirect the "Terminated" message from the shell.  SIGBUS
 # is not defined by standard C so it is used conditionally.
 
-(check_exec "$@") >> $logfile 2>&1 <> $logfile 2>&1 <
 static void sighandler(int sig){
 raise(SIGTERM);
@@ -1080,7 +1081,7 @@ check_type(){
 type=$2
 shift 2
 disable_sanitized "$type"
-check_code cc "$headers" "$type v" "$@" && enable_sanitized "$type"
+test_code cc "$headers" "$type v" "$@" && enable_sanitized "$type"
 }
 
 check_struct(){
@@ -1090,7 +1091,7 @@ check_struct(){
 member=$3
 shift 3
 disable_sanitized "${struct}_${member}"
-check_code cc "$headers" "const void *p = &(($struct *)0)->$member" "$@" &&
+test_code cc "$headers" "const void *p = &(($struct *)0)->$member" "$@" &&
 enable_sanitized "${struct}_${member}"
 }
 
@@ -1099,7 +1100,7 @@ check_builtin(){
 name=$1
 shift
 disable "$name"
-check_code ld "$@" && enable "$name"
+test_code ld "$@" && enable "$name"
 }
 
 check_compile_assert(){
@@ -1109,7 +1110,7 @@ check_compile_assert(){
 condition=$3
 shift 

Re: [libav-devel] [PATCH] Add AV1 video decoding support through libaom

[Mark Thompson]

On 10/02/18 16:29, Luca Barbato wrote:
> On 10/02/2018 16:59, Diego Biurrun wrote:
>> Looks OK in general.
>>
>> On Fri, Feb 09, 2018 at 10:51:36AM +0100, Luca Barbato wrote:
>>> --- /dev/null
>>> +++ b/libavcodec/libaom.c
>>> @@ -0,0 +1,90 @@
>>> +
>>> +#define HIGH_DEPTH(fmt)   \
>>> +    case AOM_IMG_FMT_I ## fmt ## 16: switch (depth) { \
>>> +    case 10: return AV_PIX_FMT_YUV ## fmt ## P10; \
>>> +    case 12: return AV_PIX_FMT_YUV ## fmt ## P12; \
>>> +    default: return AV_PIX_FMT_NONE;  \
>>
>> Move the switch statement to the next line for better readability please.
> 
> Sure
> 
>>
>>> +enum AVPixelFormat ff_aom_imgfmt_to_pixfmt(aom_img_fmt_t img, int depth)
>>> +{
>>> +    switch (img) {
>>> +    case AOM_IMG_FMT_RGB24: return AV_PIX_FMT_RGB24;
>>> +    case AOM_IMG_FMT_RGB565:    return AV_PIX_FMT_RGB565BE;
>>> +    case AOM_IMG_FMT_RGB555:    return AV_PIX_FMT_RGB555BE;
>>> +    case AOM_IMG_FMT_UYVY:  return AV_PIX_FMT_UYVY422;
>>> +    case AOM_IMG_FMT_YUY2:  return AV_PIX_FMT_YUYV422;
>>> +    case AOM_IMG_FMT_YVYU:  return AV_PIX_FMT_YVYU422;
>>> +    case AOM_IMG_FMT_BGR24: return AV_PIX_FMT_BGR24;
>>> +    case AOM_IMG_FMT_ARGB:  return AV_PIX_FMT_ARGB;
>>> +    case AOM_IMG_FMT_ARGB_LE:   return AV_PIX_FMT_BGRA;
>>> +    case AOM_IMG_FMT_RGB565_LE: return AV_PIX_FMT_RGB565LE;
>>> +    case AOM_IMG_FMT_RGB555_LE: return AV_PIX_FMT_RGB555LE;
>>> +    case AOM_IMG_FMT_I420:  return AV_PIX_FMT_YUV420P;
>>> +    case AOM_IMG_FMT_I422:  return AV_PIX_FMT_YUV422P;
>>> +    case AOM_IMG_FMT_I444:  return AV_PIX_FMT_YUV444P;
>>> +    case AOM_IMG_FMT_444A:  return AV_PIX_FMT_YUVA444P;
>>> +    case AOM_IMG_FMT_I440:  return AV_PIX_FMT_YUV440P;
>>
>> I'd break those lines.
> 
> I can run uncrustify to break it, is that the outcome you'd expect?
> 
>>> +/*    case AOM_IMG_FMT_I42016:    return AV_PIX_FMT_YUV420P16;
>>> +    case AOM_IMG_FMT_I42216:    return AV_PIX_FMT_YUV422P16;
>>> +    case AOM_IMG_FMT_I44416:    return AV_PIX_FMT_YUV444P16; */
>>
>> Why is this commented out?
> 
> I should just remove it, thanks for reminding me.
> 
>>> +aom_img_fmt_t ff_aom_pixfmt_to_imgfmt(enum AVPixelFormat pix)
>>> +{
>>> +    switch (pix) {
>>> +    case AV_PIX_FMT_RGB24:    return AOM_IMG_FMT_RGB24;
>>> +    case AV_PIX_FMT_RGB565BE: return AOM_IMG_FMT_RGB565;
>>> +    case AV_PIX_FMT_RGB555BE: return AOM_IMG_FMT_RGB555;
>>> +    case AV_PIX_FMT_UYVY422:  return AOM_IMG_FMT_UYVY;
>>> +    case AV_PIX_FMT_YUYV422:  return AOM_IMG_FMT_YUY2;
>>> +    case AV_PIX_FMT_YVYU422:  return AOM_IMG_FMT_YVYU;
>>> +    case AV_PIX_FMT_BGR24:    return AOM_IMG_FMT_BGR24;
>>> +    case AV_PIX_FMT_ARGB: return AOM_IMG_FMT_ARGB;
>>> +    case AV_PIX_FMT_BGRA: return AOM_IMG_FMT_ARGB_LE;
>>> +    case AV_PIX_FMT_RGB565LE: return AOM_IMG_FMT_RGB565_LE;
>>> +    case AV_PIX_FMT_RGB555LE: return AOM_IMG_FMT_RGB555_LE;
>>> +    case AV_PIX_FMT_YUV420P:  return AOM_IMG_FMT_I420;
>>> +    case AV_PIX_FMT_YUV422P:  return AOM_IMG_FMT_I422;
>>> +    case AV_PIX_FMT_YUV444P:  return AOM_IMG_FMT_I444;
>>> +    case AV_PIX_FMT_YUVA444P: return AOM_IMG_FMT_444A;
>>> +    case AV_PIX_FMT_YUV440P:  return AOM_IMG_FMT_I440;
>>> +    case AV_PIX_FMT_YUV420P10:    return AOM_IMG_FMT_I42016;
>>> +    case AV_PIX_FMT_YUV422P10:    return AOM_IMG_FMT_I42216;
>>> +    case AV_PIX_FMT_YUV444P10:    return AOM_IMG_FMT_I44416;
>>> +    case AV_PIX_FMT_YUV420P12:    return AOM_IMG_FMT_I42016;
>>> +    case AV_PIX_FMT_YUV422P12:    return AOM_IMG_FMT_I42216;
>>> +    case AV_PIX_FMT_YUV444P12:    return AOM_IMG_FMT_I44416;
>>
>> same
> 
> Ok.

If you feel like tearing down the bikeshed and building a different one: I 
think this would be nicer as a single table with the two functions reading it, 
rather than two functions which duplicate a lot of the information.

(Or ignore this if you prefer how the bikeshed is currently made...)

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

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

[Mark Thompson]

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(+)

diff --git a/libavcodec/h264_metadata_bsf.c b/libavcodec/h264_metadata_bsf.c
index 88e1a7750..f888be761 100644
--- a/libavcodec/h264_metadata_bsf.c
+++ b/libavcodec/h264_metadata_bsf.c
@@ -63,6 +63,8 @@ typedef struct H264MetadataContext {
 
 const char *sei_user_data;
 int sei_first_au;
+
+int delete_filler;
 } H264MetadataContext;
 
 
@@ -347,6 +349,44 @@ static int h264_metadata_filter(AVBSFContext *bsf, 
AVPacket *out)
 }
 }
 
+if (ctx->delete_filler) {
+for (i = 0; i < au->nb_units; i++) {
+if (au->units[i].type == H264_NAL_FILLER_DATA) {
+// Filler NAL units.
+err = ff_cbs_delete_unit(ctx->cbc, au, i);
+if (err < 0) {
+av_log(bsf, AV_LOG_ERROR, "Failed to delete "
+   "filler NAL.\n");
+goto fail;
+}
+--i;
+continue;
+}
+
+if (au->units[i].type == H264_NAL_SEI) {
+// Filler SEI messages.
+H264RawSEI *sei = au->units[i].content;
+
+for (j = 0; j < sei->payload_count; j++) {
+if (sei->payload[j].payload_type ==
+H264_SEI_TYPE_FILLER_PAYLOAD) {
+err = ff_cbs_h264_delete_sei_message(ctx->cbc, au,
+ >units[i], j);
+if (err < 0) {
+av_log(bsf, AV_LOG_ERROR, "Failed to delete "
+   "filler SEI message.\n");
+goto fail;
+}
+// Renumbering might have happened, start again at
+// the same NAL unit position.
+--i;
+break;
+}
+}
+}
+}
+}
+
 err = ff_cbs_write_packet(ctx->cbc, out, au);
 if (err < 0) {
 av_log(bsf, AV_LOG_ERROR, "Failed to write packet.\n");
@@ -467,6 +507,9 @@ static const AVOption h264_metadata_options[] = {
 { "sei_user_data", "Insert SEI user data (UUID+string)",
 OFFSET(sei_user_data), AV_OPT_TYPE_STRING, { .str = NULL } },
 
+{ "delete_filler", "Delete all filler (both NAL and SEI)",
+OFFSET(delete_filler), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 1 },
+
 { NULL }
 };
 
-- 
2.15.1

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

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

[Mark Thompson]

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];
-- 
2.15.1

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

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

[Mark Thompson]

---
 libavcodec/cbs.c   | 168 +---
 libavcodec/cbs.h   |  51 -
 libavcodec/cbs_h264.h  |   4 +
 libavcodec/cbs_h2645.c | 341 +++--
 libavcodec/cbs_h265.h  |   2 +
 libavcodec/cbs_internal.h  |   3 -
 libavcodec/cbs_mpeg2.c |  91 -
 libavcodec/cbs_mpeg2.h |   5 +
 libavcodec/cbs_mpeg2_syntax_template.c |   5 +-
 libavcodec/h264_metadata_bsf.c |   6 +-
 libavcodec/h265_metadata_bsf.c |   2 +-
 libavcodec/mpeg2_metadata_bsf.c|   3 +-
 libavcodec/vaapi_encode_h264.c |   2 +-
 libavcodec/vaapi_encode_h265.c |   2 +-
 libavcodec/vaapi_encode_mpeg2.c|   2 +-
 15 files changed, 408 insertions(+), 279 deletions(-)

diff --git a/libavcodec/cbs.c b/libavcodec/cbs.c
index a8d252f6c..9d191130f 100644
--- a/libavcodec/cbs.c
+++ b/libavcodec/cbs.c
@@ -21,6 +21,7 @@
 #include "config.h"
 
 #include "libavutil/avassert.h"
+#include "libavutil/buffer.h"
 #include "libavutil/common.h"
 
 #include "cbs.h"
@@ -95,11 +96,12 @@ void ff_cbs_close(CodedBitstreamContext **ctx_ptr)
 static void cbs_unit_uninit(CodedBitstreamContext *ctx,
 CodedBitstreamUnit *unit)
 {
-if (ctx->codec->free_unit && unit->content && !unit->content_external)
-ctx->codec->free_unit(unit);
+av_buffer_unref(>content_ref);
+unit->content = NULL;
 
-av_freep(>data);
-unit->data_size = 0;
+av_buffer_unref(>data_ref);
+unit->data = NULL;
+unit->data_size= 0;
 unit->data_bit_padding = 0;
 }
 
@@ -113,7 +115,8 @@ void ff_cbs_fragment_uninit(CodedBitstreamContext *ctx,
 av_freep(>units);
 frag->nb_units = 0;
 
-av_freep(>data);
+av_buffer_unref(>data_ref);
+frag->data = NULL;
 frag->data_size= 0;
 frag->data_bit_padding = 0;
 }
@@ -133,6 +136,9 @@ static int cbs_read_fragment_content(CodedBitstreamContext 
*ctx,
 continue;
 }
 
+av_buffer_unref(>units[i].content_ref);
+frag->units[i].content = NULL;
+
 err = ctx->codec->read_unit(ctx, >units[i]);
 if (err == AVERROR(ENOSYS)) {
 av_log(ctx->log_ctx, AV_LOG_VERBOSE,
@@ -169,6 +175,27 @@ int ff_cbs_read_extradata(CodedBitstreamContext *ctx,
 return cbs_read_fragment_content(ctx, frag);
 }
 
+static int cbs_fill_fragment_data(CodedBitstreamContext *ctx,
+  CodedBitstreamFragment *frag,
+  const uint8_t *data, size_t size)
+{
+av_assert0(!frag->data && !frag->data_ref);
+
+frag->data_ref =
+av_buffer_alloc(size + AV_INPUT_BUFFER_PADDING_SIZE);
+if (!frag->data_ref)
+return AVERROR(ENOMEM);
+
+frag->data  = frag->data_ref->data;
+frag->data_size = size;
+
+memcpy(frag->data, data, size);
+memset(frag->data + size, 0,
+   AV_INPUT_BUFFER_PADDING_SIZE);
+
+return 0;
+}
+
 int ff_cbs_read_packet(CodedBitstreamContext *ctx,
CodedBitstreamFragment *frag,
const AVPacket *pkt)
@@ -177,16 +204,24 @@ int ff_cbs_read_packet(CodedBitstreamContext *ctx,
 
 memset(frag, 0, sizeof(*frag));
 
-frag->data  = pkt->data;
-frag->data_size = pkt->size;
+if (pkt->buf) {
+frag->data_ref = av_buffer_ref(pkt->buf);
+if (!frag->data_ref)
+return AVERROR(ENOMEM);
+
+frag->data  = pkt->data;
+frag->data_size = pkt->size;
+
+} else {
+err = cbs_fill_fragment_data(ctx, frag, pkt->data, pkt->size);
+if (err < 0)
+return err;
+}
 
 err = ctx->codec->split_fragment(ctx, frag, 0);
 if (err < 0)
 return err;
 
-frag->data  = NULL;
-frag->data_size = 0;
-
 return cbs_read_fragment_content(ctx, frag);
 }
 
@@ -198,17 +233,14 @@ int ff_cbs_read(CodedBitstreamContext *ctx,
 
 memset(frag, 0, sizeof(*frag));
 
-// (We won't write to this during split.)
-frag->data  = (uint8_t*)data;
-frag->data_size = size;
+err = cbs_fill_fragment_data(ctx, frag, data, size);
+if (err < 0)
+return err;
 
 err = ctx->codec->split_fragment(ctx, frag, 0);
 if (err < 0)
 return err;
 
-frag->data  = NULL;
-frag->data_size = 0;
-
 return cbs_read_fragment_content(ctx, frag);
 }
 
@@ -219,17 +251,25 @@ int ff_cbs_write_fragment_data(CodedBitstreamContext *ctx,
 int err, i;
 
 for (i = 0; i < frag->nb_units; i++) {
-if (!frag->units[i].content)
+CodedBitstreamUnit *unit = >units[i];
+
+if (!unit->content)
 continue;
 
-err = ctx->codec->write_unit(ctx, >units[i]);
+av_buffer_unref(>data_ref);
+unit->data = NULL;
+
+err = ctx->codec->write_unit(ctx, unit);
 if (err < 0) {
 

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

[Mark Thompson]

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

diff --git a/libavcodec/cbs_h264.h b/libavcodec/cbs_h264.h
index 14ea69ae2..8c17680bb 100644
--- a/libavcodec/cbs_h264.h
+++ b/libavcodec/cbs_h264.h
@@ -22,6 +22,7 @@
 #include 
 #include 
 
+#include "cbs.h"
 #include "cbs_h2645.h"
 #include "h264.h"
 
@@ -428,4 +429,22 @@ typedef struct CodedBitstreamH264Context {
 } CodedBitstreamH264Context;
 
 
+/**
+ * Add an SEI message to an access unit.
+ */
+int ff_cbs_h264_add_sei_message(CodedBitstreamContext *ctx,
+CodedBitstreamFragment *access_unit,
+const H264RawSEIPayload *payload);
+
+/**
+ * Delete an SEI message from an access unit.
+ *
+ * Deletes from nal_unit, which must be an SEI NAL unit.  If this is the
+ * last message in nal_unit, also deletes it from access_unit.
+ */
+int ff_cbs_h264_delete_sei_message(CodedBitstreamContext *ctx,
+   CodedBitstreamFragment *access_unit,
+   CodedBitstreamUnit *nal_unit,
+   int position);
+
 #endif /* AVCODEC_CBS_H264_H */
diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c
index c98bab998..7b2e1b532 100644
--- a/libavcodec/cbs_h2645.c
+++ b/libavcodec/cbs_h2645.c
@@ -1394,3 +1394,92 @@ const CodedBitstreamType ff_cbs_type_h265 = {
 
 .close = _h265_close,
 };
+
+int ff_cbs_h264_add_sei_message(CodedBitstreamContext *ctx,
+CodedBitstreamFragment *au,
+const H264RawSEIPayload *payload)
+{
+H264RawSEI *sei;
+CodedBitstreamUnit *nal = NULL;
+int err, i;
+
+// Find an existing SEI NAL unit to add to.
+for (i = 0; i < au->nb_units; i++) {
+if (au->units[i].type == H264_NAL_SEI) {
+nal = >units[i];
+break;
+}
+}
+if (nal) {
+sei = nal->content;
+
+} else {
+// Need to make a new SEI NAL unit.  Insert it before the first
+// slice data NAL unit; if no slice data, add at the end.
+AVBufferRef *sei_ref;
+
+sei_ref = av_buffer_allocz(sizeof(*sei));
+if (!sei_ref)
+return AVERROR(ENOMEM);
+sei = (H264RawSEI*)sei_ref->data;
+
+sei->nal_unit_header.nal_unit_type = H264_NAL_SEI;
+sei->nal_unit_header.nal_ref_idc   = 0;
+
+for (i = 0; i < au->nb_units; i++) {
+if (au->units[i].type == H264_NAL_SLICE ||
+au->units[i].type == H264_NAL_IDR_SLICE)
+break;
+}
+
+err = ff_cbs_insert_unit_content(ctx, au, i, H264_NAL_SEI,
+ sei, sei_ref);
+av_buffer_unref(_ref);
+if (err < 0)
+return err;
+}
+
+if (sei->payload_count >= H264_MAX_SEI_PAYLOADS) {
+av_log(ctx->log_ctx, AV_LOG_ERROR, "Too many payloads in "
+   "SEI NAL unit.\n");
+return AVERROR(EINVAL);
+}
+
+memcpy(>payload[sei->payload_count], payload, sizeof(*payload));
+++sei->payload_count;
+
+return 0;
+}
+
+int ff_cbs_h264_delete_sei_message(CodedBitstreamContext *ctx,
+   CodedBitstreamFragment *au,
+   CodedBitstreamUnit *nal,
+   int position)
+{
+H264RawSEI *sei = nal->content;
+
+av_assert0(nal->type == H264_NAL_SEI);
+av_assert0(position >= 0 && position < sei->payload_count);
+
+if (position == 0 && sei->payload_count == 1) {
+// Deleting NAL unit entirely.
+int i;
+
+for (i = 0; i < au->nb_units; i++) {
+if (>units[i] == nal)
+break;
+}
+av_assert0(i < au->nb_units && "NAL unit not in access unit.");
+
+return ff_cbs_delete_unit(ctx, au, i);
+} else {
+cbs_h264_free_sei_payload(>payload[position]);
+
+--sei->payload_count;
+memmove(sei->payload + position,
+sei->payload + position + 1,
+(sei->payload_count - position) * sizeof(*sei->payload));
+
+return 0;
+}
+}
-- 
2.15.1

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

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

[Mark Thompson]

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) {
-- 
2.15.1

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

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

[Mark Thompson]

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

diff --git a/libavcodec/h264_metadata_bsf.c b/libavcodec/h264_metadata_bsf.c
index 6a1904e31..ce85781c6 100644
--- a/libavcodec/h264_metadata_bsf.c
+++ b/libavcodec/h264_metadata_bsf.c
@@ -290,42 +290,11 @@ 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) {
-H264RawSEI *sei;
-H264RawSEIPayload *payload;
-H264RawSEIUserDataUnregistered *udu;
-int sei_pos, sei_new;
-
-for (i = 0; i < au->nb_units; i++) {
-if (au->units[i].type == H264_NAL_SEI ||
-au->units[i].type == H264_NAL_SLICE ||
-au->units[i].type == H264_NAL_IDR_SLICE)
-break;
-}
-sei_pos = i;
-
-if (sei_pos < au->nb_units &&
-au->units[sei_pos].type == H264_NAL_SEI) {
-sei_new = 0;
-sei = au->units[sei_pos].content;
-} else {
-sei_new = 1;
-sei = >sei_nal;
-memset(sei, 0, sizeof(*sei));
-
-sei->nal_unit_header.nal_unit_type = H264_NAL_SEI;
-
-err = ff_cbs_insert_unit_content(ctx->cbc, au, sei_pos,
- H264_NAL_SEI, sei, NULL);
-if (err < 0) {
-av_log(bsf, AV_LOG_ERROR, "Failed to insert SEI.\n");
-goto fail;
-}
-}
-
-payload = >payload[sei->payload_count];
-
-payload->payload_type = H264_SEI_TYPE_USER_DATA_UNREGISTERED;
-udu = >payload.user_data_unregistered;
+H264RawSEIPayload payload = {
+.payload_type = H264_SEI_TYPE_USER_DATA_UNREGISTERED,
+};
+H264RawSEIUserDataUnregistered *udu =
+_data_unregistered;
 
 for (i = j = 0; j < 32 && ctx->sei_user_data[i]; i++) {
 int c, v;
@@ -345,21 +314,25 @@ static int h264_metadata_filter(AVBSFContext *bsf, 
AVPacket *out)
 ++j;
 }
 if (j == 32 && ctx->sei_user_data[i] == '+') {
-sei_udu_string = av_strdup(ctx->sei_user_data + i + 1);
-if (!sei_udu_string) {
+size_t len = strlen(ctx->sei_user_data + i + 1);
+
+udu->data_ref = av_buffer_alloc(len + 1);
+if (!udu->data_ref) {
 err = AVERROR(ENOMEM);
-goto sei_fail;
+goto fail;
 }
 
-udu->data = sei_udu_string;
-udu->data_length = strlen(sei_udu_string);
+udu->data= udu->data_ref->data;
+udu->data_length = len + 1;
+memcpy(udu->data, ctx->sei_user_data + i + 1, len + 1);
 
-payload->payload_size = 16 + udu->data_length;
+payload.payload_size = 16 + udu->data_length;
 
-if (!sei_new) {
-// This will be freed by the existing internal
-// reference in fragment_uninit().
-sei_udu_string = NULL;
+err = ff_cbs_h264_add_sei_message(ctx->cbc, au, );
+if (err < 0) {
+av_log(bsf, AV_LOG_ERROR, "Failed to add user data SEI "
+   "message to access unit.\n");
+goto fail;
 }
 
 } else {
@@ -367,12 +340,7 @@ static int h264_metadata_filter(AVBSFContext *bsf, 
AVPacket *out)
 av_log(bsf, AV_LOG_ERROR, "Invalid user data: "
"must be \"UUID+string\".\n");
 err = AVERROR(EINVAL);
-sei_fail:
-memset(payload, 0, sizeof(*payload));
-goto fail;
 }
-
-++sei->payload_count;
 }
 
 err = ff_cbs_write_packet(ctx->cbc, out, au);
-- 
2.15.1

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

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

[Mark Thompson]

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

diff --git a/libavcodec/cbs_h264.h b/libavcodec/cbs_h264.h
index a59b7be1b..5a7dc2769 100644
--- a/libavcodec/cbs_h264.h
+++ b/libavcodec/cbs_h264.h
@@ -408,6 +408,12 @@ typedef struct H264RawSlice {
 AVBufferRef *data_ref;
 } H264RawSlice;
 
+typedef struct H264RawFiller {
+H264RawNALUnitHeader nal_unit_header;
+
+uint32_t filler_size;
+} H264RawFiller;
+
 
 typedef struct CodedBitstreamH264Context {
 // Reader/writer context in common with the H.265 implementation.
diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c
index 7f5972cd7..bb3a11949 100644
--- a/libavcodec/cbs_h2645.c
+++ b/libavcodec/cbs_h2645.c
@@ -815,6 +815,19 @@ static int cbs_h264_read_nal_unit(CodedBitstreamContext 
*ctx,
 }
 break;
 
+case H264_NAL_FILLER_DATA:
+{
+err = ff_cbs_alloc_unit_content(ctx, unit,
+sizeof(H264RawFiller), NULL);
+if (err < 0)
+return err;
+
+err = cbs_h264_read_filler(ctx, , unit->content);
+if (err < 0)
+return err;
+}
+break;
+
 default:
 return AVERROR(ENOSYS);
 }
@@ -1070,6 +1083,14 @@ static int cbs_h264_write_nal_unit(CodedBitstreamContext 
*ctx,
 }
 break;
 
+case H264_NAL_FILLER_DATA:
+{
+err = cbs_h264_write_filler(ctx, pbc, unit->content);
+if (err < 0)
+return err;
+}
+break;
+
 default:
 av_log(ctx->log_ctx, AV_LOG_ERROR, "Write unimplemented for "
"NAL unit type %"PRIu32".\n", unit->type);
diff --git a/libavcodec/cbs_h264_syntax_template.c 
b/libavcodec/cbs_h264_syntax_template.c
index 29e973598..1aa788858 100644
--- a/libavcodec/cbs_h264_syntax_template.c
+++ b/libavcodec/cbs_h264_syntax_template.c
@@ -1247,3 +1247,32 @@ static int FUNC(slice_header)(CodedBitstreamContext 
*ctx, RWContext *rw,
 
 return 0;
 }
+
+static int FUNC(filler)(CodedBitstreamContext *ctx, RWContext *rw,
+H264RawFiller *current)
+{
+av_unused int ff_byte = 0xff;
+int err;
+
+HEADER("Filler Data");
+
+CHECK(FUNC(nal_unit_header)(ctx, rw, >nal_unit_header,
+1 << H264_NAL_FILLER_DATA));
+
+#ifdef READ
+while (bitstream_peek(rw, 8) == 0xff) {
+xu(8, ff_byte, ff_byte, 0xff, 0xff);
+++current->filler_size;
+}
+#else
+{
+uint32_t i;
+for (i = 0; i < current->filler_size; i++)
+xu(8, ff_byte, ff_byte, 0xff, 0xff);
+}
+#endif
+
+CHECK(FUNC(rbsp_trailing_bits)(ctx, rw));
+
+return 0;
+}
-- 
2.15.1

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

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

[Mark Thompson]

---
 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]) \
-- 
2.15.1

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

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

[Mark Thompson]

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(-)

diff --git a/libavcodec/cbs_h264.h b/libavcodec/cbs_h264.h
index 8c17680bb..a59b7be1b 100644
--- a/libavcodec/cbs_h264.h
+++ b/libavcodec/cbs_h264.h
@@ -195,7 +195,9 @@ typedef struct H264RawPPS {
 uint8_t slice_group_change_direction_flag;
 uint16_t slice_group_change_rate_minus1;
 uint16_t pic_size_in_map_units_minus1;
-uint8_t slice_group_id[H264_MAX_MB_PIC_SIZE];
+
+uint8_t *slice_group_id;
+AVBufferRef *slice_group_id_ref;
 
 uint8_t num_ref_idx_l0_default_active_minus1;
 uint8_t num_ref_idx_l1_default_active_minus1;
diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c
index 7b2e1b532..7f5972cd7 100644
--- a/libavcodec/cbs_h2645.c
+++ b/libavcodec/cbs_h2645.c
@@ -394,6 +394,13 @@ static int cbs_h2645_read_more_rbsp_data(BitstreamContext 
*bc)
 #undef allocate
 
 
+static void cbs_h264_free_pps(void *unit, uint8_t *content)
+{
+H264RawPPS *pps = (H264RawPPS*)content;
+av_buffer_unref(>slice_group_id_ref);
+av_freep();
+}
+
 static void cbs_h264_free_sei_payload(H264RawSEIPayload *payload)
 {
 switch (payload->payload_type) {
@@ -725,7 +732,8 @@ static int cbs_h264_read_nal_unit(CodedBitstreamContext 
*ctx,
 {
 H264RawPPS *pps;
 
-err = ff_cbs_alloc_unit_content(ctx, unit, sizeof(*pps), NULL);
+err = ff_cbs_alloc_unit_content(ctx, unit, sizeof(*pps),
+_h264_free_pps);
 if (err < 0)
 return err;
 pps = unit->content;
diff --git a/libavcodec/cbs_h264_syntax_template.c 
b/libavcodec/cbs_h264_syntax_template.c
index c2fd54682..29e973598 100644
--- a/libavcodec/cbs_h264_syntax_template.c
+++ b/libavcodec/cbs_h264_syntax_template.c
@@ -404,6 +404,9 @@ static int FUNC(pps)(CodedBitstreamContext *ctx, RWContext 
*rw,
 ue(slice_group_change_rate_minus1, 0, pic_size - 1);
 } else if (current->slice_group_map_type == 6) {
 ue(pic_size_in_map_units_minus1, pic_size - 1, pic_size - 1);
+
+allocate(current->slice_group_id,
+ current->pic_size_in_map_units_minus1 + 1);
 for (i = 0; i <= current->pic_size_in_map_units_minus1; i++)
 u(av_log2(2 * current->num_slice_groups_minus1 + 1),
   slice_group_id[i], 0, current->num_slice_groups_minus1);
-- 
2.15.1

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

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

[Mark Thompson]

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");
-- 
2.15.1

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

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

[Mark Thompson]

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
@@ -36,13 +36,17 @@ static int vaapi_encode_make_packed_header(AVCodecContext 
*avctx,
 VAAPIEncodeContext *ctx = avctx->priv_data;
 VAStatus vas;
 VABufferID param_buffer, data_buffer;
+VABufferID *tmp;
 VAEncPackedHeaderParameterBuffer params = {
 .type = type,
 .bit_length = bit_len,
 .has_emulation_bytes = 1,
 };
 
-av_assert0(pic->nb_param_buffers + 2 <= MAX_PARAM_BUFFERS);
+tmp = av_realloc_array(pic->param_buffers, sizeof(*tmp), 
pic->nb_param_buffers + 2);
+if (!tmp)
+return AVERROR(ENOMEM);
+pic->param_buffers = tmp;
 
 vas = vaCreateBuffer(ctx->hwctx->display, ctx->va_context,
  VAEncPackedHeaderParameterBufferType,
@@ -77,9 +81,13 @@ static int vaapi_encode_make_param_buffer(AVCodecContext 
*avctx,
 {
 VAAPIEncodeContext *ctx = avctx->priv_data;
 VAStatus vas;
+VABufferID *tmp;
 VABufferID buffer;
 
-av_assert0(pic->nb_param_buffers + 1 <= MAX_PARAM_BUFFERS);
+tmp = av_realloc_array(pic->param_buffers, sizeof(*tmp), 
pic->nb_param_buffers + 1);
+if (!tmp)
+return AVERROR(ENOMEM);
+pic->param_buffers = tmp;
 
 vas = vaCreateBuffer(ctx->hwctx->display, ctx->va_context,
  type, len, 1, data, );
@@ -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;
+}
 for (i = 0; i < pic->nb_slices; i++) {
-slice = av_mallocz(sizeof(*slice));
-if (!slice) {
-err = AVERROR(ENOMEM);
-goto fail;
-}
+slice = >slices[i];
 slice->index = i;
-pic->slices[i] = slice;
 
 if (ctx->codec->slice_params_size > 0) {
 slice->codec_slice_params = 
av_mallocz(ctx->codec->slice_params_size);
@@ -425,8 +432,16 @@ fail_with_picture:
 fail:
 for(i = 0; i < pic->nb_param_buffers; i++)
 vaDestroyBuffer(ctx->hwctx->display, pic->param_buffers[i]);
+for (i = 0; i < pic->nb_slices; i++) {
+if (pic->slices) {
+av_freep(>slices[i].priv_data);
+av_freep(>slices[i].codec_slice_params);
+}
+}
 fail_at_end:
 av_freep(>codec_picture_params);
+av_freep(>param_buffers);
+av_freep(>slices);
 av_frame_free(>recon_image);
 av_buffer_unref(>output_buffer_ref);
 pic->output_buffer = VA_INVALID_ID;
@@ -535,15 +550,18 @@ static int vaapi_encode_free(AVCodecContext *avctx,
 vaapi_encode_discard(avctx, pic);
 
 for (i = 0; i < pic->nb_slices; i++) {
-av_freep(>slices[i]->priv_data);
-av_freep(>slices[i]->codec_slice_params);
-av_freep(>slices[i]);
+if (pic->slices) {
+av_freep(>slices[i].priv_data);
+av_freep(>slices[i].codec_slice_params);
+}
 }
 av_freep(>codec_picture_params);
 
 av_frame_free(>input_image);
 av_frame_free(>recon_image);
 
+av_freep(>param_buffers);
+av_freep(>slices);
 // Output buffer should already be destroyed.
 av_assert0(pic->output_buffer == VA_INVALID_ID);
 
diff --git a/libavcodec/vaapi_encode.h b/libavcodec/vaapi_encode.h
index 1b0fed80e..31c379053 100644
--- a/libavcodec/vaapi_encode.h
+++ b/libavcodec/vaapi_encode.h
@@ -35,8 +35,6 @@ enum {
 MAX_CONFIG_ATTRIBUTES  = 4,
 MAX_GLOBAL_PARAMS  = 4,
 MAX_PICTURE_REFERENCES = 2,
-MAX_PICTURE_SLICES = 112,
-MAX_PARAM_BUFFERS  = 128,
 MAX_REORDER_DELAY  = 16,
 MAX_PARAM_BUFFER_SIZE  = 1024,
 };
@@ -73,7 +71,7 @@ typedef struct VAAPIEncodePicture {
 VASurfaceID recon_surface;
 
 int  nb_param_buffers;
-VABufferID  param_buffers[MAX_PARAM_BUFFERS];
+VABufferID *param_buffers;
 
 AVBufferRef*output_buffer_ref;
 VABufferID  output_buffer;
@@ -85,7 +83,7 @@ typedef struct VAAPIEncodePicture {
 struct VAAPIEncodePicture *refs[MAX_PICTURE_REFERENCES];
 
 int  nb_slices;
-VAAPIEncodeSlice *slices[MAX_PICTURE_SLICES];
+VAAPIEncodeSlice *slices;
 } VAAPIEncodePicture;
 
 typedef struct VAAPIEncodeContext {
-- 
2.15.1

___
libav-devel mailing list
libav-devel@libav.org

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

[Mark Thompson]

... 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(-)

diff --git a/libavcodec/cbs.c b/libavcodec/cbs.c
index 3baa31a4d..fd9baa299 100644
--- a/libavcodec/cbs.c
+++ b/libavcodec/cbs.c
@@ -39,9 +39,10 @@ static const CodedBitstreamType *cbs_type_table[] = {
 #endif
 };
 
-int ff_cbs_init(CodedBitstreamContext *ctx,
+int ff_cbs_init(CodedBitstreamContext **ctx_ptr,
 enum AVCodecID codec_id, void *log_ctx)
 {
+CodedBitstreamContext *ctx;
 const CodedBitstreamType *type;
 int i;
 
@@ -55,27 +56,40 @@ int ff_cbs_init(CodedBitstreamContext *ctx,
 if (!type)
 return AVERROR(EINVAL);
 
+ctx = av_mallocz(sizeof(*ctx));
+if (!ctx)
+return AVERROR(ENOMEM);
+
 ctx->log_ctx = log_ctx;
 ctx->codec   = type;
 
 ctx->priv_data = av_mallocz(ctx->codec->priv_data_size);
-if (!ctx->priv_data)
+if (!ctx->priv_data) {
+av_freep();
 return AVERROR(ENOMEM);
+}
 
 ctx->decompose_unit_types = NULL;
 
 ctx->trace_enable = 0;
 ctx->trace_level  = AV_LOG_TRACE;
 
+*ctx_ptr = ctx;
 return 0;
 }
 
-void ff_cbs_close(CodedBitstreamContext *ctx)
+void ff_cbs_close(CodedBitstreamContext **ctx_ptr)
 {
+CodedBitstreamContext *ctx = *ctx_ptr;
+
+if (!ctx)
+return;
+
 if (ctx->codec && ctx->codec->close)
 ctx->codec->close(ctx);
 
 av_freep(>priv_data);
+av_freep(ctx_ptr);
 }
 
 static void cbs_unit_uninit(CodedBitstreamContext *ctx,
diff --git a/libavcodec/cbs.h b/libavcodec/cbs.h
index 01b2239b7..34ee78be3 100644
--- a/libavcodec/cbs.h
+++ b/libavcodec/cbs.h
@@ -169,15 +169,15 @@ typedef struct CodedBitstreamContext {
 
 
 /**
- * Initialise a new context for the given codec.
+ * Create and initialise a new context for the given codec.
  */
-int ff_cbs_init(CodedBitstreamContext *ctx,
+int ff_cbs_init(CodedBitstreamContext **ctx,
 enum AVCodecID codec_id, void *log_ctx);
 
 /**
  * Close a context and free all internal state.
  */
-void ff_cbs_close(CodedBitstreamContext *ctx);
+void ff_cbs_close(CodedBitstreamContext **ctx);
 
 
 /**
diff --git a/libavcodec/h264_metadata_bsf.c b/libavcodec/h264_metadata_bsf.c
index ac0b9823b..2b579e9d3 100644
--- a/libavcodec/h264_metadata_bsf.c
+++ b/libavcodec/h264_metadata_bsf.c
@@ -35,7 +35,7 @@ enum {
 typedef struct H264MetadataContext {
 const AVClass *class;
 
-CodedBitstreamContext cbc;
+CodedBitstreamContext *cbc;
 CodedBitstreamFragment access_unit;
 
 H264RawAUD aud_nal;
@@ -214,7 +214,7 @@ static int h264_metadata_filter(AVBSFContext *bsf, AVPacket 
*out)
 if (err < 0)
 goto fail;
 
-err = ff_cbs_read_packet(>cbc, au, in);
+err = ff_cbs_read_packet(ctx->cbc, au, in);
 if (err < 0) {
 av_log(bsf, AV_LOG_ERROR, "Failed to read packet.\n");
 goto fail;
@@ -229,7 +229,7 @@ static int h264_metadata_filter(AVBSFContext *bsf, AVPacket 
*out)
 // If an AUD is present, it must be the first NAL unit.
 if (au->units[0].type == H264_NAL_AUD) {
 if (ctx->aud == REMOVE)
-ff_cbs_delete_unit(>cbc, au, 0);
+ff_cbs_delete_unit(ctx->cbc, au, 0);
 } else {
 if (ctx->aud == INSERT) {
 static const int primary_pic_type_table[] = {
@@ -269,7 +269,7 @@ static int h264_metadata_filter(AVBSFContext *bsf, AVPacket 
*out)
 aud->nal_unit_header.nal_unit_type = H264_NAL_AUD;
 aud->primary_pic_type = j;
 
-err = ff_cbs_insert_unit_content(>cbc, au,
+err = ff_cbs_insert_unit_content(ctx->cbc, au,
  0, H264_NAL_AUD, aud);
 if (err < 0) {
 av_log(bsf, AV_LOG_ERROR, "Failed to insert AUD.\n");
@@ -314,7 +314,7 @@ static int h264_metadata_filter(AVBSFContext *bsf, AVPacket 
*out)
 
 sei->nal_unit_header.nal_unit_type = H264_NAL_SEI;
 
-err = ff_cbs_insert_unit_content(>cbc, au,
+err = ff_cbs_insert_unit_content(ctx->cbc, au,
  sei_pos, H264_NAL_SEI, sei);
 if (err < 0) {
 av_log(bsf, AV_LOG_ERROR, "Failed to insert SEI.\n");
@@ 

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

[Mark Thompson]

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

diff --git a/libavcodec/cbs.h b/libavcodec/cbs.h
index 85c7b5557..ffeca057a 100644
--- a/libavcodec/cbs.h
+++ b/libavcodec/cbs.h
@@ -25,6 +25,19 @@
 #include "avcodec.h"
 
 
+/*
+ * This defines a framework for converting between a coded bitstream
+ * and structures defining all individual syntax elements found in
+ * such a stream.
+ *
+ * Conversion in both directions is possible.  Given a coded bitstream
+ * (any meaningful fragment), it can be parsed and decomposed into
+ * syntax elements stored in a set of codec-specific structures.
+ * Similarly, given a set of those same codec-specific structures the
+ * syntax elements can be serialised and combined to create a coded
+ * bitstream.
+ */
+
 struct CodedBitstreamType;
 
 /**
@@ -39,7 +52,7 @@ typedef uint32_t CodedBitstreamUnitType;
 /**
  * Coded bitstream unit structure.
  *
- * A bitstream unit the the smallest element of a bitstream which
+ * A bitstream unit the smallest element of a bitstream which
  * is meaningful on its own.  For example, an H.264 NAL unit.
  *
  * See the codec-specific header for the meaning of this for any
@@ -52,7 +65,7 @@ typedef struct CodedBitstreamUnit {
 CodedBitstreamUnitType type;
 
 /**
- * Pointer to the bitstream form of this unit.
+ * Pointer to the directly-parsable bitstream form of this unit.
  *
  * May be NULL if the unit currently only exists in decomposed form.
  */
@@ -114,7 +127,7 @@ typedef struct CodedBitstreamFragment {
 /**
  * Number of units in this fragment.
  *
- * This may be zero if the fragment only exists in bistream form
+ * This may be zero if the fragment only exists in bitstream form
  * and has not been decomposed.
  */
 int  nb_units;
@@ -162,7 +175,7 @@ typedef struct CodedBitstreamContext {
 /**
  * Length of the decompose_unit_types array.
  */
-intnb_decompose_unit_types;
+int nb_decompose_unit_types;
 
 /**
  * Enable trace output during read/write operations.
@@ -204,6 +217,10 @@ int ff_cbs_read_extradata(CodedBitstreamContext *ctx,
 /**
  * Read the data bitstream from a packet into a fragment, then
  * split into units and decompose.
+ *
+ * This also updates the internal state of the coded bitstream context
+ * with any persistent data from the fragment which may be required to
+ * read following fragments (e.g. parameter sets).
  */
 int ff_cbs_read_packet(CodedBitstreamContext *ctx,
CodedBitstreamFragment *frag,
@@ -212,6 +229,10 @@ int ff_cbs_read_packet(CodedBitstreamContext *ctx,
 /**
  * Read a bitstream from a memory region into a fragment, then
  * split into units and decompose.
+ *
+ * This also updates the internal state of the coded bitstream context
+ * with any persistent data from the fragment which may be required to
+ * read following fragments (e.g. parameter sets).
  */
 int ff_cbs_read(CodedBitstreamContext *ctx,
 CodedBitstreamFragment *frag,
@@ -225,12 +246,18 @@ int ff_cbs_read(CodedBitstreamContext *ctx,
  * data buffer.  When modifying the content of decomposed units, this
  * can be used to regenerate the bitstream form of units or the whole
  * fragment so that it can be extracted for other use.
+ *
+ * This also updates the internal state of the coded bitstream context
+ * with any persistent data from the fragment which may be required to
+ * write following fragments (e.g. parameter sets).
  */
 int ff_cbs_write_fragment_data(CodedBitstreamContext *ctx,
CodedBitstreamFragment *frag);
 
 /**
  * Write the bitstream of a fragment to the extradata in codec parameters.
+ *
+ * This replaces any existing extradata in the structure.
  */
 int ff_cbs_write_extradata(CodedBitstreamContext *ctx,
AVCodecParameters *par,
diff --git a/libavcodec/cbs_internal.h b/libavcodec/cbs_internal.h
index dddeae9d5..1a1c22f06 100644
--- a/libavcodec/cbs_internal.h
+++ b/libavcodec/cbs_internal.h
@@ -32,6 +32,9 @@ typedef struct CodedBitstreamType {
 
 // Split frag->data into coded bitstream units, creating the
 // frag->units array.  Fill data but not content on each unit.
+// The header argument should be set if the fragment came from
+// a header block, which may require different parsing for some
+// codecs (e.g. the AVCC header in H.264).
 int (*split_fragment)(CodedBitstreamContext *ctx,
   CodedBitstreamFragment *frag,
   int header);
-- 
2.15.1

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

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

[Mark Thompson]

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.


 libavcodec/vaapi_encode.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
index 8cba847f7..7063c6877 100644
--- a/libavcodec/vaapi_encode.c
+++ b/libavcodec/vaapi_encode.c
@@ -1554,6 +1554,8 @@ av_cold int ff_vaapi_encode_close(AVCodecContext *avctx)
 vaapi_encode_free(avctx, pic);
 }
 
+av_buffer_pool_uninit(>output_buffer_pool);
+
 if (ctx->va_context != VA_INVALID_ID) {
 vaDestroyContext(ctx->hwctx->display, ctx->va_context);
 ctx->va_context = VA_INVALID_ID;
@@ -1564,8 +1566,6 @@ av_cold int ff_vaapi_encode_close(AVCodecContext *avctx)
 ctx->va_config = VA_INVALID_ID;
 }
 
-av_buffer_pool_uninit(>output_buffer_pool);
-
 av_freep(>codec_sequence_params);
 av_freep(>codec_picture_params);
 
-- 
2.15.1

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

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

[Mark Thompson]

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(-)

diff --git a/libavcodec/cbs.c b/libavcodec/cbs.c
index fd9baa299..e5819afce 100644
--- a/libavcodec/cbs.c
+++ b/libavcodec/cbs.c
@@ -137,10 +137,10 @@ static int 
cbs_read_fragment_content(CodedBitstreamContext *ctx,
 if (err == AVERROR(ENOSYS)) {
 av_log(ctx->log_ctx, AV_LOG_WARNING,
"Decomposition unimplemented for unit %d "
-   "(type %d).\n", i, frag->units[i].type);
+   "(type %"PRIu32").\n", i, frag->units[i].type);
 } else if (err < 0) {
 av_log(ctx->log_ctx, AV_LOG_ERROR, "Failed to read unit %d "
-   "(type %d).\n", i, frag->units[i].type);
+   "(type %"PRIu32").\n", i, frag->units[i].type);
 return err;
 }
 }
@@ -225,7 +225,7 @@ int ff_cbs_write_fragment_data(CodedBitstreamContext *ctx,
 err = ctx->codec->write_unit(ctx, >units[i]);
 if (err < 0) {
 av_log(ctx->log_ctx, AV_LOG_ERROR, "Failed to write unit %d "
-   "(type %d).\n", i, frag->units[i].type);
+   "(type %"PRIu32").\n", i, frag->units[i].type);
 return err;
 }
 }
@@ -421,7 +421,8 @@ static int cbs_insert_unit(CodedBitstreamContext *ctx,
 
 int ff_cbs_insert_unit_content(CodedBitstreamContext *ctx,
CodedBitstreamFragment *frag,
-   int position, uint32_t type,
+   int position,
+   CodedBitstreamUnitType type,
void *content)
 {
 int err;
@@ -443,7 +444,8 @@ int ff_cbs_insert_unit_content(CodedBitstreamContext *ctx,
 
 int ff_cbs_insert_unit_data(CodedBitstreamContext *ctx,
 CodedBitstreamFragment *frag,
-int position, uint32_t type,
+int position,
+CodedBitstreamUnitType type,
 uint8_t *data, size_t data_size)
 {
 int err;
diff --git a/libavcodec/cbs.h b/libavcodec/cbs.h
index 34ee78be3..85c7b5557 100644
--- a/libavcodec/cbs.h
+++ b/libavcodec/cbs.h
@@ -27,6 +27,15 @@
 
 struct CodedBitstreamType;
 
+/**
+ * The codec-specific type of a bitstream unit.
+ *
+ * H.264 / AVC: nal_unit_type
+ * H.265 / HEVC: nal_unit_type
+ * MPEG-2: start code value (without prefix)
+ */
+typedef uint32_t CodedBitstreamUnitType;
+
 /**
  * Coded bitstream unit structure.
  *
@@ -40,7 +49,7 @@ typedef struct CodedBitstreamUnit {
 /**
  * Codec-specific type of this unit.
  */
-uint32_t type;
+CodedBitstreamUnitType type;
 
 /**
  * Pointer to the bitstream form of this unit.
@@ -149,7 +158,7 @@ typedef struct CodedBitstreamContext {
  * Types not in this list will be available in bitstream form only.
  * If NULL, all supported types will be decomposed.
  */
-uint32_t *decompose_unit_types;
+CodedBitstreamUnitType *decompose_unit_types;
 /**
  * Length of the decompose_unit_types array.
  */
@@ -250,7 +259,8 @@ void ff_cbs_fragment_uninit(CodedBitstreamContext *ctx,
  */
 int ff_cbs_insert_unit_content(CodedBitstreamContext *ctx,
CodedBitstreamFragment *frag,
-   int position, uint32_t type,
+   int position,
+   CodedBitstreamUnitType type,
void *content);
 
 /**
@@ -260,7 +270,8 @@ int ff_cbs_insert_unit_content(CodedBitstreamContext *ctx,
  */
 int ff_cbs_insert_unit_data(CodedBitstreamContext *ctx,
 CodedBitstreamFragment *frag,
-int position, uint32_t type,
+int position,
+CodedBitstreamUnitType type,
 uint8_t *data, size_t data_size);
 
 /**
diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c
index 00eed0f28..e3b5bf618 100644
--- a/libavcodec/cbs_h2645.c
+++ b/libavcodec/cbs_h2645.c
@@ -1213,7 +1213,7 @@ static int cbs_h265_write_nal_unit(CodedBitstreamContext 
*ctx,
 
 default:
 av_log(ctx->log_ctx, AV_LOG_ERROR, "Write unimplemented for "
-   "NAL unit type %d.\n", unit->type);
+   "NAL unit type %"PRIu32".\n", unit->type);
 return AVERROR_PATCHWELCOME;
 }
 
diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c
index 54875c2e1..5956f3933 100644
--- a/libavcodec/cbs_mpeg2.c
+++ b/libavcodec/cbs_mpeg2.c
@@ -220,7 +220,7 @@ static int cbs_mpeg2_read_unit(CodedBitstreamContext *ctx,
 START(0xb8, MPEG2RawGroupOfPicturesHeader, 
group_of_pictures_header);
 

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

[Mark Thompson]

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;
 }
-- 
2.15.1

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

Re: [libav-devel] Add HW H.264 and HEVC encoding for AMD GPUs based on AMF SDK

[Diego Biurrun]

On Fri, Feb 09, 2018 at 08:23:28PM +, Mironov, Mikhail wrote:
> > From: libav-devel [mailto:libav-devel-boun...@libav.org] On Behalf Of Diego
> > On Wed, Feb 07, 2018 at 09:46:29AM +0100, Diego Biurrun wrote:
> > > On Sun, Feb 04, 2018 at 07:04:39PM +, Mironov, Mikhail wrote:
> > > > I have a developer who will cover AMF integration on regular basis
> > (together with me). He starts tomorrow and will be up to date soon.
> > > > I will ask him to cleanup the things you mentioned if you will not 
> > > > submit
> > the changes first.
> > > > He is on BCC for now.
> > >
> > > It seems a bit more cleanup is needed on your side as well:
> > >
> > > /home/libav/libs/include/AMF/core/Platform.h:431:16: error: implicit
> > declaration of function ‘memcmp’ [-Werror=implicit-function-declaration]
> > >
> > > Indeed that header is missing a string.h #include and things break on one
> > of my systems.
> > 
> > I worked around that locally and added FATE (compile) testing for AMF:
> > 
> > https://fate.libav.org/x86_64-linux-external-libs/20180209180449
> 
> We will push this fix to AMF repo with upcoming update  - hopefully next week.

Please include the patch I sent while you're at it. :)

Let me know when the fix is public so I can get rid of my local workaround.

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

Re: [libav-devel] Add HW H.264 and HEVC encoding for AMD GPUs based on AMF SDK

[Diego Biurrun]

On Thu, Feb 08, 2018 at 03:17:03PM +0100, Diego Biurrun wrote:
> On Wed, Feb 07, 2018 at 06:39:39PM +, Mironov, Mikhail wrote:
> > I quickly searched but did not find any formal standard on include guards. 
> > I did see some recommendations in forums though.
> 
> I suggest searching the C99 standard instead of random forums:
> 
> ISO/IEC 9899:1999 (E)
> 7.1.3 Reserved identifiers
> — All identifiers that begin with an underscore and either an uppercase 
> letter or another
> underscore are always reserved for any use.
> 
> > To get some statistics: I checked few header files in Windows SDK, OpenCL 
> > SDK, OpenGL, Intel Media SDK - I see all kind of styles including  
> > __FILE_H__.
> > For example: 
> > https://github.com/Intel-Media-SDK/MediaSDK/blob/master/api/include/mfxvideo.h
> 
> Yes, bad practices abound. People look at kernel headers or libc headers
> and wrongly believe that that is the example to follow. Then others look
> at other projects for inspiration and cargo cult the error around.
> 
> Please be a good example and fix it instead of perpetuating this
> malpractice. It's as easy as
> 
>   find . -name \*.h | xargs sed -i -e 's/ __AMF\(.*\)__/ AMF_\1/'
> 
> from the root of the AMF source tree or the include dir (requires GNU sed).

Did that for you, please apply the attached patch.

Diego
>From 42544648c332fa48d846632cf1d1f38571171379 Mon Sep 17 00:00:00 2001
From: Diego Biurrun 
Date: Sun, 11 Feb 2018 18:51:39 +0100
Subject: [PATCH] Do not use C99-reserved identifiers as multiple inclusion
 guards

Identifiers starting with __ or _ and a capital letter are reserved,
see section 7.1.3 Reserved identifiers of the C99 standard.
---
 amf/public/include/components/Ambisonic2SRenderer.h  | 6 +++---
 amf/public/include/components/AudioCapture.h | 6 +++---
 amf/public/include/components/Component.h| 6 +++---
 amf/public/include/components/ComponentCaps.h| 6 +++---
 amf/public/include/components/DisplayCapture.h   | 6 +++---
 amf/public/include/components/FFMPEGAudioConverter.h | 6 +++---
 amf/public/include/components/FFMPEGAudioDecoder.h   | 6 +++---
 amf/public/include/components/FFMPEGAudioEncoder.h   | 6 +++---
 amf/public/include/components/FFMPEGComponents.h | 6 +++---
 amf/public/include/components/FFMPEGFileDemuxer.h| 6 +++---
 amf/public/include/components/FFMPEGFileMuxer.h  | 6 +++---
 amf/public/include/components/MediaSource.h  | 6 +++---
 amf/public/include/components/VideoConverter.h   | 6 +++---
 amf/public/include/components/VideoEncoderHEVC.h | 6 +++---
 amf/public/include/components/VideoEncoderVCE.h  | 6 +++---
 amf/public/include/core/AudioBuffer.h| 6 +++---
 amf/public/include/core/Buffer.h | 6 +++---
 amf/public/include/core/Compute.h| 6 +++---
 amf/public/include/core/ComputeFactory.h | 7 ---
 amf/public/include/core/Context.h| 6 +++---
 amf/public/include/core/Data.h   | 6 +++---
 amf/public/include/core/Debug.h  | 6 +++---
 amf/public/include/core/Dump.h   | 4 ++--
 amf/public/include/core/Factory.h| 4 ++--
 amf/public/include/core/Interface.h  | 6 +++---
 amf/public/include/core/Plane.h  | 6 +++---
 amf/public/include/core/Platform.h   | 6 +++---
 amf/public/include/core/PropertyStorage.h| 6 +++---
 amf/public/include/core/PropertyStorageEx.h  | 6 +++---
 amf/public/include/core/Result.h | 6 +++---
 amf/public/include/core/Surface.h| 6 +++---
 amf/public/include/core/Trace.h  | 6 +++---
 amf/public/include/core/Variant.h| 6 +++---
 amf/public/include/core/Version.h| 6 +++---
 34 files changed, 101 insertions(+), 100 deletions(-)

diff --git a/amf/public/include/components/Ambisonic2SRenderer.h b/amf/public/include/components/Ambisonic2SRenderer.h
index af9c882..46bc623 100644
--- a/amf/public/include/components/Ambisonic2SRenderer.h
+++ b/amf/public/include/components/Ambisonic2SRenderer.h
@@ -34,8 +34,8 @@
 // interface declaration;  Ambisonic to Stereo Renderer
 //-
 
-#ifndef __AMFAmbisonic2SRendererHW__h__
-#define __AMFAmbisonic2SRendererHW__h__
+#ifndef AMF_Ambisonic2SRenderer_h
+#define AMF_Ambisonic2SRenderer_h
 #pragma once
 
 #include "public/include/components/Component.h"
@@ -76,4 +76,4 @@ extern "C"
 {
 AMF_RESULT AMF_CDECL_CALL AMFCreateComponentAmbisonic(amf::AMFContext* pContext, void* reserved, amf::AMFComponent** ppComponent);
 }
-#endif //#ifndef __AMFAmbisonic2SRendererHW__h__
+#endif //#ifndef AMF_Ambisonic2SRenderer_h
diff --git a/amf/public/include/components/AudioCapture.h b/amf/public/include/components/AudioCapture.h

Re: [libav-devel] [PATCH] Add AV1 video decoding support through libaom

[Luca Barbato]

On 11/02/2018 17:50, Diego Biurrun wrote:

On Fri, Feb 09, 2018 at 10:51:36AM +0100, Luca Barbato wrote:

  libavcodec/version.h   |   2 +-

--- a/libavcodec/version.h
+++ b/libavcodec/version.h
@@ -28,7 +28,7 @@

  #define LIBAVCODEC_VERSION_MAJOR 58
-#define LIBAVCODEC_VERSION_MINOR  8
+#define LIBAVCODEC_VERSION_MINOR  9
  #define LIBAVCODEC_VERSION_MICRO  0


I guess we should decide what to do about bumping version when there
is no change to installed headers, e.g. when a decoder is added without
adding an AV_CODEC_ID. Anton has been saying that minor should not be
bumped in those cases and I think he is right.


I do not have opinion about it, locally amended to avoid the version bump.

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

Re: [libav-devel] [PATCH] Add AV1 video decoding support through libaom

[Diego Biurrun]

On Fri, Feb 09, 2018 at 10:51:36AM +0100, Luca Barbato wrote:
>  libavcodec/version.h   |   2 +-
> 
> --- a/libavcodec/version.h
> +++ b/libavcodec/version.h
> @@ -28,7 +28,7 @@
> 
>  #define LIBAVCODEC_VERSION_MAJOR 58
> -#define LIBAVCODEC_VERSION_MINOR  8
> +#define LIBAVCODEC_VERSION_MINOR  9
>  #define LIBAVCODEC_VERSION_MICRO  0

I guess we should decide what to do about bumping version when there
is no change to installed headers, e.g. when a decoder is added without
adding an AV_CODEC_ID. Anton has been saying that minor should not be
bumped in those cases and I think he is right.

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

Re: [libav-devel] [PATCH] Add AV1 video decoding support through libaom

[Luca Barbato]

On 10/02/2018 18:15, Diego Biurrun wrote:

On Sat, Feb 10, 2018 at 05:29:52PM +0100, Luca Barbato wrote:

On 10/02/2018 16:59, Diego Biurrun wrote:

On Fri, Feb 09, 2018 at 10:51:36AM +0100, Luca Barbato wrote:

--- /dev/null
+++ b/libavcodec/libaom.c
@@ -0,0 +1,90 @@
+enum AVPixelFormat ff_aom_imgfmt_to_pixfmt(aom_img_fmt_t img, int depth)
+{
+switch (img) {
+case AOM_IMG_FMT_RGB24: return AV_PIX_FMT_RGB24;
+case AOM_IMG_FMT_RGB565:return AV_PIX_FMT_RGB565BE;
+case AOM_IMG_FMT_RGB555:return AV_PIX_FMT_RGB555BE;
+case AOM_IMG_FMT_UYVY:  return AV_PIX_FMT_UYVY422;
+case AOM_IMG_FMT_YUY2:  return AV_PIX_FMT_YUYV422;
+case AOM_IMG_FMT_YVYU:  return AV_PIX_FMT_YVYU422;
+case AOM_IMG_FMT_BGR24: return AV_PIX_FMT_BGR24;
+case AOM_IMG_FMT_ARGB:  return AV_PIX_FMT_ARGB;
+case AOM_IMG_FMT_ARGB_LE:   return AV_PIX_FMT_BGRA;
+case AOM_IMG_FMT_RGB565_LE: return AV_PIX_FMT_RGB565LE;
+case AOM_IMG_FMT_RGB555_LE: return AV_PIX_FMT_RGB555LE;
+case AOM_IMG_FMT_I420:  return AV_PIX_FMT_YUV420P;
+case AOM_IMG_FMT_I422:  return AV_PIX_FMT_YUV422P;
+case AOM_IMG_FMT_I444:  return AV_PIX_FMT_YUV444P;
+case AOM_IMG_FMT_444A:  return AV_PIX_FMT_YUVA444P;
+case AOM_IMG_FMT_I440:  return AV_PIX_FMT_YUV440P;


I'd break those lines.


I can run uncrustify to break it, is that the outcome you'd expect?


That would do the trick, yes.


--- /dev/null
+++ b/libavcodec/libaomdec.c
@@ -0,0 +1,137 @@
+static av_cold int aom_init(AVCodecContext *avctx,
+const struct aom_codec_iface *iface)
+{
+if (aom_codec_dec_init(>decoder, iface, , 0) != AOM_CODEC_OK) {
+const char *error = aom_codec_error(>decoder);
+av_log(avctx, AV_LOG_ERROR, "Failed to initialize decoder: %s\n",
+   error);
+return AVERROR(EINVAL);


These don't look like user-supplied values, so I think EINVAL is not the
right error code.


suggest one :)


Never mind, these values are sort of user-supplied.


+AVCodec ff_libaom_av1_decoder = {
+.init   = av1_init,
+.close  = aom_free,
+.decode = aom_decode,


Why av1_init and not aom_init?


Ideally if we want to support av2 through libaom we'd just have to have a
av2_init.


That's a little bit too future-proof for my taste. If and when that day
arrives, it's trivial to rename the function. Who knows, maybe there will
not even be two init functions then..


Comments addressed, I'll push it tomorrow.

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

Re: [libav-devel] [PATCH] lavc/qsv: default la_ds to MFX_LOOKAHEAD_DS_UNKNOWN

[Luca Barbato]

On 11/02/2018 13:40, Maxym Dmytrychenko wrote:

looks reasonable.



+1

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

Re: [libav-devel] [PATCH] lavc/qsv: default la_ds to MFX_LOOKAHEAD_DS_UNKNOWN

[Maxym Dmytrychenko]

looks reasonable.

On Sun, Feb 11, 2018 at 8:43 AM, Ruiling Song 
wrote:

> MFX_LOOKAHEAD_DS_UNKNOWN means auto.
> -1 is not a valid value.
>
> Signed-off-by: Ruiling Song 
> ---
>  libavcodec/qsvenc.h | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/libavcodec/qsvenc.h b/libavcodec/qsvenc.h
> index 088a61d..6a8f119 100644
> --- a/libavcodec/qsvenc.h
> +++ b/libavcodec/qsvenc.h
> @@ -71,8 +71,9 @@
>  { "slow",   NULL, 0, AV_OPT_TYPE_CONST, { .i64 =
> MFX_TARGETUSAGE_BEST_QUALITY  }, INT_MIN, INT_MAX, VE, "preset" },
>\
>  { "la_depth", "Number of frames to analyze before encoding.",
> OFFSET(qsv.la_depth), AV_OPT_TYPE_INT, { .i64 = 9 },   9, 100, VE,
> "la_depth" },  \
>  { "unset", NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 9 }, INT_MIN, INT_MAX,
>VE, "la_depth" },  \
> -{ "la_ds", "Downscaling factor for the frames saved for the lookahead
> analysis", OFFSET(qsv.la_ds), AV_OPT_TYPE_INT, { .i64 = -1 },   -1,
> MFX_LOOKAHEAD_DS_4x, VE, "la_ds" }, \
> -{ "auto", NULL, 0, AV_OPT_TYPE_CONST, { .i64 = -1 }, INT_MIN, INT_MAX,
>VE, "la_ds" }, \
> +{ "la_ds", "Downscaling factor for the frames saved for the lookahead
> analysis", OFFSET(qsv.la_ds), AV_OPT_TYPE_INT,\
> +{ .i64 = MFX_LOOKAHEAD_DS_UNKNOWN },
> MFX_LOOKAHEAD_DS_UNKNOWN, MFX_LOOKAHEAD_DS_4x, VE, "la_ds" },
> \
> +{ "auto", NULL, 0, AV_OPT_TYPE_CONST, { .i64 = MFX_LOOKAHEAD_DS_UNKNOWN
> }, INT_MIN, INT_MAX, VE, "la_ds" }, \
>  { "off", NULL, 0, AV_OPT_TYPE_CONST, { .i64 = MFX_LOOKAHEAD_DS_OFF },
> INT_MIN, INT_MAX,  VE, "la_ds" }, \
>  { "2x", NULL, 0, AV_OPT_TYPE_CONST, { .i64 = MFX_LOOKAHEAD_DS_2x },
> INT_MIN, INT_MAX,VE, "la_ds" },
> \
>  { "4x", NULL, 0, AV_OPT_TYPE_CONST, { .i64 = MFX_LOOKAHEAD_DS_4x },
> INT_MIN, INT_MAX,VE, "la_ds" },
> \
> --
> 2.7.4
>
> ___
> 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