Re: [libav-devel] [PATCH 5/5] aarch64: vp9itxfm: Do separate functions for half/quarter idct16 and idct32 (alternative 2)

2017-02-08 Thread Janne Grunau
On 2017-02-06 00:16:41 +0200, Martin Storsjö wrote:
> 
> Ok, so after running a slightly shorter clip (which seems to have about as
> large percentage of runtime doing IDCT as the previous one) with a bit more
> iterations, I've got the following results (the 'user' part from 'time
> avconv -threads 1 -i foo -f null -'):
> 
> 32 orig   32 alt1   32 alt2   64 orig   64 alt1   64 alt2
> 40.436s   40.148s   40.008s   37.428s   37.356s   37.192s
> 40.596s   40.140s   40.216s   37.572s   37.524s   37.384s
> 40.512s   40.228s   40.188s   37.740s   37.588s   37.368s
> 40.584s   40.136s   40.216s   37.880s   37.492s   37.348s
> 40.572s   40.292s   40.232s   37.756s   37.556s   37.676s
> 40.764s   40.312s   40.232s   37.876s   37.640s   37.468s
> 40.688s   40.284s   40.368s   37.972s   37.608s   37.460s
> 
> So while alt2 is faster in most runs, the margin is not quite as big as in
> the previous benchmark. (The benchmarks were done on a practically unloaded
> system so it shouldn't vary too much from run to run, but in practice, the
> first few runs seem to be slightly faster than the later ones.)
> 
> I.e. around 400 ms gain out of 40 s for alt1, and then another -50 - +150 ms
> speedup on top of that for alt2.
> 
> What do you think?

At least it looks like the difference between alt1 and alt2 are quite 
similar on 32- and 64-bit. So we should use the same variant on both 
archs. I favor alternate 2.

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

Re: [libav-devel] [PATCH 5/5] aarch64: vp9itxfm: Do a simpler half/quarter idct16/idct32 when possible (alternative 1)

2017-02-08 Thread Martin Storsjö

On Thu, 9 Feb 2017, Janne Grunau wrote:


On 2017-02-05 14:05:49 +0200, Martin Storsjö wrote:

On Sun, 5 Feb 2017, Janne Grunau wrote:

>> // out1 = in1 + in2
>> // out2 = in1 - in2
>> .macro butterfly_8h out1, out2, in1, in2
>>@@ -463,7 +510,7 @@ function idct16x16_dc_add_neon
>> ret
>> endfunc
>>
>>-function idct16
>>+.macro idct16_full
>> dmbutterfly0v16, v24, v16, v24, v2, v3, v4, v5, v6, v7 // v16 = 
t0a,  v24 = t1a
>> dmbutterfly v20, v28, v0.h[1], v0.h[2], v2, v3, v4, v5 // v20 = 
t2a,  v28 = t3a
>> dmbutterfly v18, v30, v0.h[3], v0.h[4], v2, v3, v4, v5 // v18 = 
t4a,  v30 = t7a
>>@@ -485,7 +532,10 @@ function idct16
>> dmbutterfly0v22, v26, v22, v26, v2, v3, v18, v19, v30, v31   
 // v22 = t6a,  v26 = t5a
>> dmbutterfly v23, v25, v0.h[1], v0.h[2], v18, v19, v30, v31   
 // v23 = t9a,  v25 = t14a
>> dmbutterfly v27, v21, v0.h[1], v0.h[2], v18, v19, v30, v31, 
neg=1 // v27 = t13a, v21 = t10a
>>+idct16_end
>
>I think it would be clearer if idct16_end is used directly from the macro.
>it would probably also make sense to move idct16_end and avoid the
>idct16_full macro. The patch might be smaller and it is immediately
>obvious that there is no code change but the resulting code is more
>comlicated than it needs to be. same applies to arm if we go with
>alternative 1.

Ok, so you mean like this?

function idct16
dmbutterfly...

idct16_end
endfunc


that would be one option, the other would be to move the idct_end 
instructions as a macro out of the the existing idct16 function and use 
it as macro. That would make the full idct structural identical to the 
half and quarter version and avoid a macro only used once.


I'm not really following what you're suggesting here - can you outline it 
with a code sample like mine above?


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

Re: [libav-devel] [PATCH 5/5] aarch64: vp9itxfm: Do a simpler half/quarter idct16/idct32 when possible (alternative 1)

2017-02-08 Thread Janne Grunau
On 2017-02-05 14:05:49 +0200, Martin Storsjö wrote:
> On Sun, 5 Feb 2017, Janne Grunau wrote:
> 
> >> // out1 = in1 + in2
> >> // out2 = in1 - in2
> >> .macro butterfly_8h out1, out2, in1, in2
> >>@@ -463,7 +510,7 @@ function idct16x16_dc_add_neon
> >> ret
> >> endfunc
> >>
> >>-function idct16
> >>+.macro idct16_full
> >> dmbutterfly0v16, v24, v16, v24, v2, v3, v4, v5, v6, v7 // v16 
> >> = t0a,  v24 = t1a
> >> dmbutterfly v20, v28, v0.h[1], v0.h[2], v2, v3, v4, v5 // v20 
> >> = t2a,  v28 = t3a
> >> dmbutterfly v18, v30, v0.h[3], v0.h[4], v2, v3, v4, v5 // v18 
> >> = t4a,  v30 = t7a
> >>@@ -485,7 +532,10 @@ function idct16
> >> dmbutterfly0v22, v26, v22, v26, v2, v3, v18, v19, v30, v31 
> >>// v22 = t6a,  v26 = t5a
> >> dmbutterfly v23, v25, v0.h[1], v0.h[2], v18, v19, v30, v31 
> >>// v23 = t9a,  v25 = t14a
> >> dmbutterfly v27, v21, v0.h[1], v0.h[2], v18, v19, v30, v31, 
> >> neg=1 // v27 = t13a, v21 = t10a
> >>+idct16_end
> >
> >I think it would be clearer if idct16_end is used directly from the macro.
> >it would probably also make sense to move idct16_end and avoid the
> >idct16_full macro. The patch might be smaller and it is immediately
> >obvious that there is no code change but the resulting code is more
> >comlicated than it needs to be. same applies to arm if we go with
> >alternative 1.
> 
> Ok, so you mean like this?
> 
> function idct16
> dmbutterfly...
> 
> idct16_end
> endfunc

that would be one option, the other would be to move the idct_end 
instructions as a macro out of the the existing idct16 function and use 
it as macro. That would make the full idct structural identical to the 
half and quarter version and avoid a macro only used once.

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

Re: [libav-devel] [PATCH 1/2] intmath: add faster clz support

2017-02-08 Thread Luca Barbato
On 03/02/2017 18:00, Vittorio Giovara wrote:
> From: Ganesh Ajjanagadde 
> 
> ---
> One of the api requirements for pixlet.
> Vittorio
> 
>  libavutil/intmath.h | 19 +++
>  1 file changed, 19 insertions(+)
> 

Possibly OK.

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

Re: [libav-devel] [PATCH] build: Move cli tool sources to a separate subdirectory

2017-02-08 Thread Luca Barbato
On 05/02/2017 16:44, Diego Biurrun wrote:
> Generates the binaries in the top-level dir now.

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

Re: [libav-devel] [PATCH] avcodec/nvenc: make gpu indices independent of supported capabilities

2017-02-08 Thread Luca Barbato
On 08/02/2017 23:52, Ganapathy Raman Kasi wrote:
> Hi,
> 
> This patch fixes multiple unnecessary cuda contexts which are created
> incase the gpu device to use is greater than 0. Each cuda context
> creation takes about 100ms and this patch helps in reducing the
> initialization time incase we are using one of the secondary gpus in
> the system. The patch is being ported to libav. Please let me know if
> there is a better way to port patches. Thanks.

Looks good, thank you! and sending like this is ok, if you could use git
send-email it might be faster, but I guess depends on your mailing system.

lu



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

[libav-devel] [PATCH] avcodec/nvenc: make gpu indices independent of supported capabilities

2017-02-08 Thread Ganapathy Raman Kasi
Hi,

This patch fixes multiple unnecessary cuda contexts which are created incase 
the gpu device to use is greater than 0. Each cuda context creation takes about 
100ms and this patch helps in reducing the initialization time incase we are 
using one of the secondary gpus in the system. The patch is being ported to 
libav. Please let me know if there is a better way to port patches. Thanks.

Regards
Ganapathy

---
This email message is for the sole use of the intended recipient(s) and may 
contain
confidential information.  Any unauthorized review, use, disclosure or 
distribution
is prohibited.  If you are not the intended recipient, please contact the 
sender by
reply email and destroy all copies of the original message.
---


0001-avcodec-nvenc-make-gpu-indices-independent-of-suppor.patch
Description: 0001-avcodec-nvenc-make-gpu-indices-independent-of-suppor.patch
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] imgutils: Document av_image_get_buffer_size()

2017-02-08 Thread Luca Barbato
On 08/02/2017 15:48, Vittorio Giovara wrote:
> On Tue, Feb 7, 2017 at 10:01 AM, Vittorio Giovara
>  wrote:
>> ---
>>  libavutil/imgutils.h | 7 ++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/libavutil/imgutils.h b/libavutil/imgutils.h
>> index 67063a2..ac1bcb8 100644
>> --- a/libavutil/imgutils.h
>> +++ b/libavutil/imgutils.h
>> @@ -169,7 +169,12 @@ int av_image_fill_arrays(uint8_t *dst_data[4], int 
>> dst_linesize[4],
>>   * Return the size in bytes of the amount of data required to store an
>>   * image with the given parameters.
>>   *
>> - * @param[in] align the assumed linesize alignment
>> + * @param pix_fmt  the pixel format of the image
>> + * @param widththe width of the image in pixels
>> + * @param height   the height of the image in pixels
>> + * @param alignthe assumed linesize alignment
>> + * @return the size in bytes required for src, a negative error code
>> + * in case of failure
> 
> I was pointed to the fact that there is no `src` field, I'll replace
> the @return line with
>  * @return the buffer size in bytes, a negative error code in case of failure
> 
Ok.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] avcodec: Mark some codecs with threadsafe init as such

2017-02-08 Thread Luca Barbato
On 08/02/2017 15:42, Derek Buitenhuis wrote:
> Signed-off-by: Derek Buitenhuis 
> ---
> Circa November 2015.
> ---

Sure, why not?

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

[libav-devel] [PATCH] h264dec: fix dropped initial SEI recovery point

2017-02-08 Thread John Stebbins
---
 libavcodec/h264dec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c
index 5137039..6d7aa7b 100644
--- a/libavcodec/h264dec.c
+++ b/libavcodec/h264dec.c
@@ -452,7 +452,6 @@ void ff_h264_flush_change(H264Context *h)
 if (h->cur_pic_ptr)
 h->cur_pic_ptr->reference = 0;
 h->first_field = 0;
-ff_h264_sei_uninit(>sei);
 h->recovery_frame = -1;
 h->frame_recovered = 0;
 }
@@ -466,6 +465,7 @@ static void flush_dpb(AVCodecContext *avctx)
 memset(h->delayed_pic, 0, sizeof(h->delayed_pic));
 
 ff_h264_flush_change(h);
+ff_h264_sei_uninit(>sei);
 
 for (i = 0; i < H264_MAX_PICTURE_COUNT; i++)
 ff_h264_unref_picture(h, >DPB[i]);
-- 
2.9.3

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

Re: [libav-devel] [PATCH] configure: Correctly recurse in do_check_deps()

2017-02-08 Thread Diego Biurrun
On Wed, Feb 08, 2017 at 08:56:56PM +0200, Martin Storsjö wrote:
> On Wed, 8 Feb 2017, Diego Biurrun wrote:
> 
> >Fixes all sorts of configuration problems introducec by dad7a9c7c0ae
> >on non-Linux, non-vanilla configs. Also removes a line made redundant
> >in that commit.
> >---
> 
> Also occurred on linux/omx configs (for dlopen). I'm not sure if you meant
> non-linux && non-vanilla, or non-linux || non-vanilla :P

The latter; log message locally amended.

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

Re: [libav-devel] [PATCH 3/5] frame: allow align=0 (meaning automatic) for av_frame_get_buffer()

2017-02-08 Thread Anton Khirnov
Quoting Vittorio Giovara (2017-02-08 14:40:45)
> On Wed, Feb 8, 2017 at 5:50 AM, Anton Khirnov  wrote:
> > This will avoid every caller from hardcoding some specific alignment,
> > which may break in the future with new instruction sets.
> > ---
> >  doc/APIchanges  | 4 
> >  libavutil/frame.c   | 4 
> >  libavutil/frame.h   | 4 +++-
> >  libavutil/version.h | 2 +-
> >  4 files changed, 12 insertions(+), 2 deletions(-)
> >
> > diff --git a/doc/APIchanges b/doc/APIchanges
> > index acd1536..fd751f3 100644
> > --- a/doc/APIchanges
> > +++ b/doc/APIchanges
> > @@ -13,6 +13,10 @@ libavutil: 2015-08-28
> >
> >  API changes, most recent first:
> >
> > +2017-02-xx - xxx - lavu 55.31.1 - frame.h
> > +  Allow passing the value of 0 (meaning "automatic") as the required 
> > alignment
> > +  to av_frame_get_buffer().
> > +
> >  2017-02-xx - xxx - lavu 55.31.0 - cpu.h
> >Add av_cpu_max_align() for querying maximum required data alignment.
> >
> > diff --git a/libavutil/frame.c b/libavutil/frame.c
> > index aafaa57..aa5820c 100644
> > --- a/libavutil/frame.c
> > +++ b/libavutil/frame.c
> > @@ -19,6 +19,7 @@
> >  #include "channel_layout.h"
> >  #include "buffer.h"
> >  #include "common.h"
> > +#include "cpu.h"
> >  #include "dict.h"
> >  #include "frame.h"
> >  #include "imgutils.h"
> > @@ -103,6 +104,9 @@ static int get_video_buffer(AVFrame *frame, int align)
> >  if (ret < 0)
> >  return ret;
> >
> > +if (align <= 0)
> > +align = av_cpu_max_align();
> 
> should you maybe be stricter here and check for == 0, failing for negative?

More code for little gain IMO.

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

Re: [libav-devel] [PATCH 4/6] avconv_vaapi: Use device-only setup

2017-02-08 Thread Anton Khirnov
Quoting Mark Thompson (2017-02-02 13:55:18)
> ---
> The device setup here changes because we have to set the device earlier (see 
> patch 2/6).  Because of this, -hwaccel_device is no longer supported.
> 
> Also, we have to eliminate the hwaccel_get_buffer callback because 
> AVCodecContext.hw_frames_ctx is no longer user-accessible.
> 
> 
>  avconv_vaapi.c | 145 
> +++--
>  1 file changed, 6 insertions(+), 139 deletions(-)

So much code eliminated, nice.

Patch looks good.

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

Re: [libav-devel] [PATCH 3/6] vaapi: Implement device-only setup

2017-02-08 Thread Anton Khirnov
Quoting Mark Thompson (2017-02-02 13:54:39)
> In this case, the user only supplies a device and the frame context
> is allocated internally by lavc.
> ---
> This is identical to 2/5 in the previous version (now it supports a bit more 
> than the specified API, but irrelevantly).
> 
> 
>  libavcodec/vaapi_decode.c | 132 
> --
>  libavcodec/vaapi_decode.h |   3 ++
>  2 files changed, 118 insertions(+), 17 deletions(-)
> 
> diff --git a/libavcodec/vaapi_decode.c b/libavcodec/vaapi_decode.c
> index 42f03ab..3ba0790 100644
> --- a/libavcodec/vaapi_decode.c
> +++ b/libavcodec/vaapi_decode.c
> @@ -18,6 +18,7 @@
>  
>  #include "libavutil/avassert.h"
>  #include "libavutil/common.h"
> +#include "libavutil/pixdesc.h"
>  
>  #include "avcodec.h"
>  #include "internal.h"
> @@ -280,6 +281,7 @@ static int vaapi_decode_make_config(AVCodecContext *avctx)
>  const AVCodecDescriptor *codec_desc;
>  VAProfile profile, *profile_list = NULL;
>  int profile_count, exact_match, alt_profile;
> +const AVPixFmtDescriptor *sw_desc, *desc;
>  
>  // Allowing a profile mismatch can be useful because streams may
>  // over-declare their required capabilities - in particular, many
> @@ -373,7 +375,9 @@ static int vaapi_decode_make_config(AVCodecContext *avctx)
>  goto fail;
>  }
>  
> -hwconfig = av_hwdevice_hwconfig_alloc(ctx->frames->device_ref);
> +hwconfig = av_hwdevice_hwconfig_alloc(avctx->hw_device_ctx ?
> +  avctx->hw_device_ctx :
> +  ctx->frames->device_ref);
>  if (!hwconfig) {
>  err = AVERROR(ENOMEM);
>  goto fail;
> @@ -381,24 +385,77 @@ static int vaapi_decode_make_config(AVCodecContext 
> *avctx)
>  hwconfig->config_id = ctx->va_config;
>  
>  constraints =
> -av_hwdevice_get_hwframe_constraints(ctx->frames->device_ref,
> +av_hwdevice_get_hwframe_constraints(avctx->hw_device_ctx ?
> +avctx->hw_device_ctx :
> +ctx->frames->device_ref,
>  hwconfig);
>  if (!constraints) {
> -// Ignore.
> -} else {
> -if (avctx->coded_width  < constraints->min_width  ||
> -avctx->coded_height < constraints->min_height ||
> -avctx->coded_width  > constraints->max_width  ||
> -avctx->coded_height > constraints->max_height) {
> -av_log(avctx, AV_LOG_ERROR, "Hardware does not support image "
> -   "size %dx%d (constraints: width %d-%d height %d-%d).\n",
> -   avctx->coded_width, avctx->coded_height,
> -   constraints->min_width,  constraints->max_width,
> -   constraints->min_height, constraints->max_height);
> -err = AVERROR(EINVAL);
> -goto fail;
> +err = AVERROR(ENOMEM);
> +goto fail;
> +}
> +
> +if (avctx->coded_width  < constraints->min_width  ||
> +avctx->coded_height < constraints->min_height ||
> +avctx->coded_width  > constraints->max_width  ||
> +avctx->coded_height > constraints->max_height) {
> +av_log(avctx, AV_LOG_ERROR, "Hardware does not support image "
> +   "size %dx%d (constraints: width %d-%d height %d-%d).\n",
> +   avctx->coded_width, avctx->coded_height,
> +   constraints->min_width,  constraints->max_width,
> +   constraints->min_height, constraints->max_height);
> +err = AVERROR(EINVAL);
> +goto fail;
> +}
> +if (!constraints->valid_sw_formats ||
> +constraints->valid_sw_formats[0] == AV_PIX_FMT_NONE) {
> +av_log(avctx, AV_LOG_ERROR, "Hardware does not offer any "
> +   "usable surface formats.\n");
> +err = AVERROR(EINVAL);
> +goto fail;
> +}
> +
> +// Find the first format in the list which matches the expected
> +// bit depth and subsampling.  If none are found (this can happen
> +// when 10-bit streams are decoded to 8-bit surfaces, for example)
> +// then just take the first format on the list.
> +ctx->surface_format = constraints->valid_sw_formats[0];
> +sw_desc = av_pix_fmt_desc_get(avctx->sw_pix_fmt);
> +for (i = 0; constraints->valid_sw_formats[i] != AV_PIX_FMT_NONE; i++) {
> +desc = av_pix_fmt_desc_get(constraints->valid_sw_formats[i]);
> +if (desc->nb_components != sw_desc->nb_components ||
> +desc->log2_chroma_w != sw_desc->log2_chroma_w ||
> +desc->log2_chroma_h != sw_desc->log2_chroma_h)
> +continue;
> +for (j = 0; j < desc->nb_components; j++) {
> +if (desc->comp[j].depth != sw_desc->comp[j].depth)
> +break;
>  }
> +if (j < desc->nb_components)
> +continue;
> +ctx->surface_format = 

Re: [libav-devel] [PATCH] configure: Correctly recurse in do_check_deps()

2017-02-08 Thread Martin Storsjö

On Wed, 8 Feb 2017, Diego Biurrun wrote:


Fixes all sorts of configuration problems introducec by dad7a9c7c0ae
on non-Linux, non-vanilla configs. Also removes a line made redundant
in that commit.
---


Also occurred on linux/omx configs (for dlopen). I'm not sure if you meant 
non-linux && non-vanilla, or non-linux || non-vanilla :P




Improved log message, removed a (now) stray line.

Nonetheless, please shoot me.

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

diff --git a/configure b/configure
index 7d39aca..9ebc3bf 100755
--- a/configure
+++ b/configure
@@ -612,7 +612,6 @@ is_in(){

do_check_deps(){
for cfg; do
-cfg="${cfg#!}"
enabled ${cfg}_checking && die "Circular dependency for $cfg."
disabled ${cfg}_checking && continue
enable ${cfg}_checking
@@ -627,7 +626,7 @@ do_check_deps(){
eval dep_ifn="\$${cfg}_if_any"

pushvar cfg dep_all dep_any dep_con dep_sel dep_sgs dep_ifa dep_ifn
-check_deps $dep_all $dep_any $dep_con $dep_sel $dep_sgs $dep_ifa 
$dep_ifn
+do_check_deps $dep_all $dep_any $dep_con $dep_sel $dep_sgs $dep_ifa 
$dep_ifn
popvar cfg dep_all dep_any dep_con dep_sel dep_sgs dep_ifa dep_ifn

[ -n "$dep_ifa" ] && { enabled_all $dep_ifa && enable_weak $cfg; }
--
2.1.4


OK with me, tested that it fixes the issues I saw on both linux with omx 
enabled and osx with avfoundation.


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

[libav-devel] [PATCH] build: Fine-grained link-time dependency settings

2017-02-08 Thread Diego Biurrun
Previously, all link-time dependencies were added for all libraries,
resulting in bogus link-time dependencies since not all dependencies
are shared across libraries. Also, in some cases like libavutil, not
all dependencies were taken into account, resulting in some cases of
underlinking.

To address all this mess a machinery is added for tracking which
dependency belongs to which library component and then leveraged
to determine correct dependencies for all individual libraries.
---

Rebased on top of the (do_)check_deps typo fix.

 Makefile|   2 +-
 avbuild/common.mak  |   2 +-
 avbuild/library.mak |   2 +-
 configure   | 165 +++-
 tests/checkasm/Makefile |   2 +-
 5 files changed, 124 insertions(+), 49 deletions(-)

diff --git a/Makefile b/Makefile
index 98eb3ab..90caf0f 100644
--- a/Makefile
+++ b/Makefile
@@ -118,7 +118,7 @@ FF_STATIC_DEP_LIBS := $(STATIC_DEP_LIBS)
 all: $(AVPROGS)
 
 $(TOOLS): %$(EXESUF): %.o
-   $(LD) $(LDFLAGS) $(LDEXEFLAGS) $(LD_O) $^ $(EXTRALIBS) $(ELIBS)
+   $(LD) $(LDFLAGS) $(LDEXEFLAGS) $(LD_O) $^ $(EXTRALIBS-$(*F)) 
$(EXTRALIBS) $(ELIBS)
 
 CONFIGURABLE_COMPONENTS =   \
 $(wildcard $(FFLIBS:%=$(SRC_PATH)/lib%/all*.c)) \
diff --git a/avbuild/common.mak b/avbuild/common.mak
index 236380e..a627dc0 100644
--- a/avbuild/common.mak
+++ b/avbuild/common.mak
@@ -8,7 +8,7 @@ OBJS  += $(OBJS-yes)
 FFLIBS:= $($(NAME)_FFLIBS) $(FFLIBS-yes) $(FFLIBS)
 TESTPROGS += $(TESTPROGS-yes)
 
-FFEXTRALIBS := $(FFLIBS:%=$(LD_LIB)) $(EXTRALIBS)
+FFEXTRALIBS := $(FFLIBS:%=$(LD_LIB)) $(foreach lib,EXTRALIBS-$(NAME) 
$(FFLIBS:%=EXTRALIBS-%),$($(lib))) $(EXTRALIBS)
 
 OBJS  := $(sort $(OBJS:%=$(SUBDIR)%))
 TESTOBJS  := $(TESTOBJS:%=$(SUBDIR)tests/%) $(TESTPROGS:%=$(SUBDIR)tests/%.o)
diff --git a/avbuild/library.mak b/avbuild/library.mak
index e5f6d7d..be6098c 100644
--- a/avbuild/library.mak
+++ b/avbuild/library.mak
@@ -30,7 +30,7 @@ $(TOOLS): THISLIB = $(NAME:%=$(LD_LIB))
 $(TESTPROGS): THISLIB = $(SUBDIR)$(LIBNAME)
 
 $(TESTPROGS) $(TOOLS): %$(EXESUF): %.o
-   $$(LD) $(LDFLAGS) $(LDEXEFLAGS) $$(LD_O) $$(filter %.o,$$^) $$(THISLIB) 
$(FFEXTRALIBS) $$(ELIBS)
+   $$(LD) $(LDFLAGS) $(LDEXEFLAGS) $$(LD_O) $$(filter %.o,$$^) $$(THISLIB) 
$(FFEXTRALIBS) $$(EXTRALIBS-$$(*F)) $$(ELIBS)
 
 $(SUBDIR)lib$(NAME).version: $(SUBDIR)version.h | $(SUBDIR)
$$(M) $$(SRC_PATH)/avbuild/libversion.sh $(NAME) $$< > $$@
diff --git a/configure b/configure
index bd35e49..b5a1c7d 100755
--- a/configure
+++ b/configure
@@ -610,7 +610,7 @@ is_in(){
 return 1
 }
 
-do_check_deps(){
+check_deps(){
 for cfg; do
 enabled ${cfg}_checking && die "Circular dependency for $cfg."
 disabled ${cfg}_checking && continue
@@ -626,7 +626,7 @@ do_check_deps(){
 eval dep_ifn="\$${cfg}_if_any"
 
 pushvar cfg dep_all dep_any dep_con dep_sel dep_sgs dep_ifa dep_ifn
-do_check_deps $dep_all $dep_any $dep_con $dep_sel $dep_sgs $dep_ifa 
$dep_ifn
+check_deps $dep_all $dep_any $dep_con $dep_sel $dep_sgs $dep_ifa 
$dep_ifn
 popvar cfg dep_all dep_any dep_con dep_sel dep_sgs dep_ifa dep_ifn
 
 [ -n "$dep_ifa" ] && { enabled_all $dep_ifa && enable_weak $cfg; }
@@ -639,24 +639,17 @@ do_check_deps(){
 if enabled $cfg; then
 enable_deep $dep_sel
 enable_deep_weak $dep_sgs
+for dep in $dep_all $dep_any $dep_sgs; do
+# filter out library deps, these do not belong in extralibs
+is_in $dep $LIBRARY_LIST && continue
+enabled $dep && eval append ${cfg}_extralibs ${dep}_extralibs
+done
 fi
 
 disable ${cfg}_checking
 done
 }
 
-check_deps(){
-unset allopts
-
-do_check_deps "$@"
-
-for cfg in $allopts; do
-enabled $cfg || continue
-eval dep_extralibs="\$${cfg}_extralibs"
-test -n "$dep_extralibs" && add_extralibs $dep_extralibs
-done
-}
-
 print_config(){
 pfx=$1
 files=$2
@@ -711,6 +704,15 @@ unique(){
 eval "$var=\"${uniq_list}\""
 }
 
+resolve(){
+var=$1
+tmpvar=
+for entry in $(eval echo \$$var); do
+tmpvar="$tmpvar $(eval echo \$${entry})"
+done
+eval "$var=\"${tmpvar}\""
+}
+
 add_cppflags(){
 append CPPFLAGS "$@"
 }
@@ -747,6 +749,12 @@ add_extralibs(){
 prepend extralibs $($ldflags_filter "$@")
 }
 
+add_extralibs_component(){
+component=$1
+shift
+prepend extralibs_${component} $($ldflags_filter "$@")
+}
+
 add_host_cppflags(){
 append host_cppflags "$@"
 }
@@ -1015,7 +1023,7 @@ check_lib(){
 shift 3
 disable $name
 check_func_headers "$headers" "$funcs" "$@" &&
-enable $name && add_extralibs "$@"
+enable $name && eval ${name}_extralibs="\$@"
 }
 
 check_pkg_config(){
@@ -1142,7 +1150,7 @@ require_pkg_config(){
 test "$name" = "" && name=$pkg
 

[libav-devel] [PATCH] configure: Correctly recurse in do_check_deps()

2017-02-08 Thread Diego Biurrun
Fixes all sorts of configuration problems introducec by dad7a9c7c0ae
on non-Linux, non-vanilla configs. Also removes a line made redundant
in that commit.
---

Improved log message, removed a (now) stray line.

Nonetheless, please shoot me.

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

diff --git a/configure b/configure
index 7d39aca..9ebc3bf 100755
--- a/configure
+++ b/configure
@@ -612,7 +612,6 @@ is_in(){
 
 do_check_deps(){
 for cfg; do
-cfg="${cfg#!}"
 enabled ${cfg}_checking && die "Circular dependency for $cfg."
 disabled ${cfg}_checking && continue
 enable ${cfg}_checking
@@ -627,7 +626,7 @@ do_check_deps(){
 eval dep_ifn="\$${cfg}_if_any"
 
 pushvar cfg dep_all dep_any dep_con dep_sel dep_sgs dep_ifa dep_ifn
-check_deps $dep_all $dep_any $dep_con $dep_sel $dep_sgs $dep_ifa 
$dep_ifn
+do_check_deps $dep_all $dep_any $dep_con $dep_sel $dep_sgs $dep_ifa 
$dep_ifn
 popvar cfg dep_all dep_any dep_con dep_sel dep_sgs dep_ifa dep_ifn
 
 [ -n "$dep_ifa" ] && { enabled_all $dep_ifa && enable_weak $cfg; }
-- 
2.1.4

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

[libav-devel] [PATCH] configure: Correctly recurse in do_check_deps()

2017-02-08 Thread Diego Biurrun
Fixes all sorts of configuration problems on non-Linux, non-vanilla configs.
---

Please shoot me.

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

diff --git a/configure b/configure
index 7d39aca..6f2ab7b 100755
--- a/configure
+++ b/configure
@@ -627,7 +627,7 @@ do_check_deps(){
 eval dep_ifn="\$${cfg}_if_any"
 
 pushvar cfg dep_all dep_any dep_con dep_sel dep_sgs dep_ifa dep_ifn
-check_deps $dep_all $dep_any $dep_con $dep_sel $dep_sgs $dep_ifa 
$dep_ifn
+do_check_deps $dep_all $dep_any $dep_con $dep_sel $dep_sgs $dep_ifa 
$dep_ifn
 popvar cfg dep_all dep_any dep_con dep_sel dep_sgs dep_ifa dep_ifn
 
 [ -n "$dep_ifa" ] && { enabled_all $dep_ifa && enable_weak $cfg; }
-- 
2.1.4

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

[libav-devel] [PATCH] configure: Correctly recurse in do_check_deps()

2017-02-08 Thread Diego Biurrun
---
 configure | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure b/configure
index 7d39aca..6f2ab7b 100755
--- a/configure
+++ b/configure
@@ -627,7 +627,7 @@ do_check_deps(){
 eval dep_ifn="\$${cfg}_if_any"
 
 pushvar cfg dep_all dep_any dep_con dep_sel dep_sgs dep_ifa dep_ifn
-check_deps $dep_all $dep_any $dep_con $dep_sel $dep_sgs $dep_ifa 
$dep_ifn
+do_check_deps $dep_all $dep_any $dep_con $dep_sel $dep_sgs $dep_ifa 
$dep_ifn
 popvar cfg dep_all dep_any dep_con dep_sel dep_sgs dep_ifa dep_ifn
 
 [ -n "$dep_ifa" ] && { enabled_all $dep_ifa && enable_weak $cfg; }
-- 
2.1.4

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

Re: [libav-devel] [PATCH] imgutils: Document av_image_get_buffer_size()

2017-02-08 Thread Vittorio Giovara
On Tue, Feb 7, 2017 at 10:01 AM, Vittorio Giovara
 wrote:
> ---
>  libavutil/imgutils.h | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/libavutil/imgutils.h b/libavutil/imgutils.h
> index 67063a2..ac1bcb8 100644
> --- a/libavutil/imgutils.h
> +++ b/libavutil/imgutils.h
> @@ -169,7 +169,12 @@ int av_image_fill_arrays(uint8_t *dst_data[4], int 
> dst_linesize[4],
>   * Return the size in bytes of the amount of data required to store an
>   * image with the given parameters.
>   *
> - * @param[in] align the assumed linesize alignment
> + * @param pix_fmt  the pixel format of the image
> + * @param widththe width of the image in pixels
> + * @param height   the height of the image in pixels
> + * @param alignthe assumed linesize alignment
> + * @return the size in bytes required for src, a negative error code
> + * in case of failure

I was pointed to the fact that there is no `src` field, I'll replace
the @return line with
 * @return the buffer size in bytes, a negative error code in case of failure
-- 
Vittorio
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

[libav-devel] [PATCH] avcodec: Mark some codecs with threadsafe init as such

2017-02-08 Thread Derek Buitenhuis
Signed-off-by: Derek Buitenhuis 
---
Circa November 2015.
---
 libavcodec/aasc.c  | 1 +
 libavcodec/aic.c   | 1 +
 libavcodec/anm.c   | 1 +
 libavcodec/ansi.c  | 1 +
 libavcodec/aura.c  | 1 +
 libavcodec/avs.c   | 1 +
 libavcodec/bethsoftvideo.c | 1 +
 libavcodec/bfi.c   | 1 +
 libavcodec/bmvvideo.c  | 1 +
 libavcodec/c93.c   | 1 +
 libavcodec/cllc.c  | 1 +
 libavcodec/cyuv.c  | 2 ++
 libavcodec/fraps.c | 1 +
 libavcodec/lcldec.c| 2 ++
 libavcodec/pngdec.c| 1 +
 libavcodec/r210dec.c   | 2 ++
 libavcodec/utvideodec.c| 1 +
 libavcodec/vble.c  | 1 +
 libavcodec/zerocodec.c | 1 +
 19 files changed, 22 insertions(+)

diff --git a/libavcodec/aasc.c b/libavcodec/aasc.c
index e65ea39..c4800f0 100644
--- a/libavcodec/aasc.c
+++ b/libavcodec/aasc.c
@@ -119,4 +119,5 @@ AVCodec ff_aasc_decoder = {
 .close  = aasc_decode_end,
 .decode = aasc_decode_frame,
 .capabilities   = AV_CODEC_CAP_DR1,
+.caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE,
 };
diff --git a/libavcodec/aic.c b/libavcodec/aic.c
index ed0be44..de9d7de 100644
--- a/libavcodec/aic.c
+++ b/libavcodec/aic.c
@@ -488,4 +488,5 @@ AVCodec ff_aic_decoder = {
 .decode = aic_decode_frame,
 .capabilities   = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_FRAME_THREADS,
 .init_thread_copy = ONLY_IF_THREADS_ENABLED(aic_decode_init),
+.caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE,
 };
diff --git a/libavcodec/anm.c b/libavcodec/anm.c
index b70d220..af8d843 100644
--- a/libavcodec/anm.c
+++ b/libavcodec/anm.c
@@ -199,4 +199,5 @@ AVCodec ff_anm_decoder = {
 .close  = decode_end,
 .decode = decode_frame,
 .capabilities   = AV_CODEC_CAP_DR1,
+.caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE,
 };
diff --git a/libavcodec/ansi.c b/libavcodec/ansi.c
index 65e2e16..0bdbdbe 100644
--- a/libavcodec/ansi.c
+++ b/libavcodec/ansi.c
@@ -451,4 +451,5 @@ AVCodec ff_ansi_decoder = {
 .close  = decode_close,
 .decode = decode_frame,
 .capabilities   = AV_CODEC_CAP_DR1,
+.caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE,
 };
diff --git a/libavcodec/aura.c b/libavcodec/aura.c
index a1ef6f8..6a03f8f 100644
--- a/libavcodec/aura.c
+++ b/libavcodec/aura.c
@@ -107,4 +107,5 @@ AVCodec ff_aura2_decoder = {
 .init   = aura_decode_init,
 .decode = aura_decode_frame,
 .capabilities   = AV_CODEC_CAP_DR1,
+.caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE,
 };
diff --git a/libavcodec/avs.c b/libavcodec/avs.c
index 959f570..edd91ef 100644
--- a/libavcodec/avs.c
+++ b/libavcodec/avs.c
@@ -186,4 +186,5 @@ AVCodec ff_avs_decoder = {
 .decode = avs_decode_frame,
 .close  = avs_decode_end,
 .capabilities   = AV_CODEC_CAP_DR1,
+.caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE,
 };
diff --git a/libavcodec/bethsoftvideo.c b/libavcodec/bethsoftvideo.c
index 11e2cfa..61f098b 100644
--- a/libavcodec/bethsoftvideo.c
+++ b/libavcodec/bethsoftvideo.c
@@ -164,4 +164,5 @@ AVCodec ff_bethsoftvid_decoder = {
 .close  = bethsoftvid_decode_end,
 .decode = bethsoftvid_decode_frame,
 .capabilities   = AV_CODEC_CAP_DR1,
+.caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE,
 };
diff --git a/libavcodec/bfi.c b/libavcodec/bfi.c
index 8335e9d..0ce73b1 100644
--- a/libavcodec/bfi.c
+++ b/libavcodec/bfi.c
@@ -181,4 +181,5 @@ AVCodec ff_bfi_decoder = {
 .close  = bfi_decode_close,
 .decode = bfi_decode_frame,
 .capabilities   = AV_CODEC_CAP_DR1,
+.caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE,
 };
diff --git a/libavcodec/bmvvideo.c b/libavcodec/bmvvideo.c
index 4fb42f0..698bc56 100644
--- a/libavcodec/bmvvideo.c
+++ b/libavcodec/bmvvideo.c
@@ -289,4 +289,5 @@ AVCodec ff_bmv_video_decoder = {
 .init   = decode_init,
 .decode = decode_frame,
 .capabilities   = AV_CODEC_CAP_DR1,
+.caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE,
 };
diff --git a/libavcodec/c93.c b/libavcodec/c93.c
index 18df958..e751483 100644
--- a/libavcodec/c93.c
+++ b/libavcodec/c93.c
@@ -261,4 +261,5 @@ AVCodec ff_c93_decoder = {
 .close  = decode_end,
 .decode = decode_frame,
 .capabilities   = AV_CODEC_CAP_DR1,
+.caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE,
 };
diff --git a/libavcodec/cllc.c b/libavcodec/cllc.c
index bac2b73..3c476f7 100644
--- a/libavcodec/cllc.c
+++ b/libavcodec/cllc.c
@@ -488,4 +488,5 @@ AVCodec ff_cllc_decoder = {
 .decode = cllc_decode_frame,
 .close  = cllc_decode_close,
 .capabilities   = AV_CODEC_CAP_DR1,
+.caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE,
 };
diff --git a/libavcodec/cyuv.c b/libavcodec/cyuv.c
index 86f7aac..2c4f98d 100644
--- a/libavcodec/cyuv.c
+++ b/libavcodec/cyuv.c
@@ -173,6 +173,7 @@ AVCodec ff_aura_decoder = {
 .init   = cyuv_decode_init,
 

Re: [libav-devel] [PATCH 3/5] frame: allow align=0 (meaning automatic) for av_frame_get_buffer()

2017-02-08 Thread Vittorio Giovara
On Wed, Feb 8, 2017 at 5:50 AM, Anton Khirnov  wrote:
> This will avoid every caller from hardcoding some specific alignment,
> which may break in the future with new instruction sets.
> ---
>  doc/APIchanges  | 4 
>  libavutil/frame.c   | 4 
>  libavutil/frame.h   | 4 +++-
>  libavutil/version.h | 2 +-
>  4 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/doc/APIchanges b/doc/APIchanges
> index acd1536..fd751f3 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -13,6 +13,10 @@ libavutil: 2015-08-28
>
>  API changes, most recent first:
>
> +2017-02-xx - xxx - lavu 55.31.1 - frame.h
> +  Allow passing the value of 0 (meaning "automatic") as the required 
> alignment
> +  to av_frame_get_buffer().
> +
>  2017-02-xx - xxx - lavu 55.31.0 - cpu.h
>Add av_cpu_max_align() for querying maximum required data alignment.
>
> diff --git a/libavutil/frame.c b/libavutil/frame.c
> index aafaa57..aa5820c 100644
> --- a/libavutil/frame.c
> +++ b/libavutil/frame.c
> @@ -19,6 +19,7 @@
>  #include "channel_layout.h"
>  #include "buffer.h"
>  #include "common.h"
> +#include "cpu.h"
>  #include "dict.h"
>  #include "frame.h"
>  #include "imgutils.h"
> @@ -103,6 +104,9 @@ static int get_video_buffer(AVFrame *frame, int align)
>  if (ret < 0)
>  return ret;
>
> +if (align <= 0)
> +align = av_cpu_max_align();

should you maybe be stricter here and check for == 0, failing for negative?
-- 
Vittorio
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] asfdec: Account for different Format Data sizes

2017-02-08 Thread Diego Biurrun
On Wed, Feb 08, 2017 at 12:51:37PM +0100, Alexandra Hájková wrote:
> Some muxers may use the BMP_HEADER Format Data size instead
> of the ASF-specific one.
> 
> Bug-Id: 1020
> ---

Again: CC: libav-sta...@libav.org

> --- a/libavformat/asfdec.c
> +++ b/libavformat/asfdec.c
> @@ -691,20 +691,22 @@ static int asf_read_properties(AVFormatContext *s, 
> const GUIDParseTable *g)
>  
>  static int parse_video_info(AVIOContext *pb, AVStream *st)
>  {
> -uint16_t size;
> +uint16_t size_asf; // ASF specific Format Data size
> +uint32_t size_bmp; // BMP_HEADER specific Format Data size

foo-specific ..

> --- a/libavformat/riffdec.c
> +++ b/libavformat/riffdec.c
> @@ -180,10 +180,12 @@ enum AVCodecID ff_wav_codec_get_id(unsigned int tag, 
> int bps)
>  return id;
>  }
>  
> -int ff_get_bmp_header(AVIOContext *pb, AVStream *st)
> +int ff_get_bmp_header(AVIOContext *pb, AVStream *st, uint32_t *size)
>  {
>  int tag1;
> -avio_rl32(pb); /* size */
> +uint32_t size_ = avio_rl32(pb); /* size */

Again: The comment adds no information.

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

Re: [libav-devel] [PATCH 3/5] frame: allow align=0 (meaning automatic) for av_frame_get_buffer()

2017-02-08 Thread wm4
On Wed, 08 Feb 2017 12:22:24 +0100
Anton Khirnov  wrote:

> Quoting wm4 (2017-02-08 12:08:18)
> > On Wed,  8 Feb 2017 11:50:58 +0100
> > Anton Khirnov  wrote:
> >   
> > > This will avoid every caller from hardcoding some specific alignment,
> > > which may break in the future with new instruction sets.
> > > ---  
> >   
> > >   * @param frame frame in which to store the new buffers.
> > > - * @param align required buffer size alignment
> > > + * @param align Required buffer size alignment. If equal to 0, alignment 
> > > will be
> > > + *  chosen automatically for the current CPU. It is highly
> > > + *  recommended to pass 0 here unless you know what you are 
> > > doing.
> > >   *
> > >   * @return 0 on success, a negative AVERROR on error.
> > >   */  
> > 
> > This implicitly changes the meaning of "0" from no alignment
> > (equivalent to 1) to some random alignment. I don't think that's very
> > safe. Why not make -1 the magic value that enables automatic alignment?  
> 
> 0 is not "no alignment", 0 is invalid for video and I think will do
> something random right now. For audio, 0 already means automatic. So I'm
> not changing the behaviour of any valid code.
> 

That's weird, but fine then I guess.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH 4/5] vf_interlace/x86: use symbolic names for better readability

2017-02-08 Thread Luca Barbato
On 08/02/2017 11:50, Anton Khirnov wrote:
> ---
>  libavfilter/x86/vf_interlace.asm | 30 +++---
>  1 file changed, 15 insertions(+), 15 deletions(-)
> 

Possibly ok.

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

Re: [libav-devel] [PATCH 2/5] lavc: use av_cpu_max_align() instead of hardcoding alignment requirements

2017-02-08 Thread Luca Barbato
On 08/02/2017 11:50, Anton Khirnov wrote:
> ---
>  libavcodec/utils.c | 11 ++-
>  1 file changed, 2 insertions(+), 9 deletions(-)
> 

Ok.

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

Re: [libav-devel] [PATCH 1/5] cpu: add a function for querying maximum required data alignment

2017-02-08 Thread Luca Barbato
On 08/02/2017 11:50, Anton Khirnov wrote:
> ---
>  doc/APIchanges  |  3 +++
>  libavutil/cpu.c | 13 +
>  libavutil/cpu.h | 13 +
>  libavutil/version.h |  2 +-
>  4 files changed, 30 insertions(+), 1 deletion(-)
> 

Sounds ok.

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

[libav-devel] [PATCH] hlsenc: Correctly write down all 16 bytes in hex

2017-02-08 Thread Luca Barbato
---

 libavformat/hlsenc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
index 05c9adb..3496bdd 100644
--- a/libavformat/hlsenc.c
+++ b/libavformat/hlsenc.c
@@ -106,7 +106,7 @@ static int dict_set_bin(AVDictionary **dict, const char 
*key, uint8_t *buf)
 {
 char hex[33];

-ff_data_to_hex(hex, buf, sizeof(buf), 0);
+ff_data_to_hex(hex, buf, 16, 0);
 hex[32] = '\0';

 return av_dict_set(dict, key, hex, 0);
--
2.9.2

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

[libav-devel] [PATCH] asfdec: Account for different Format Data sizes

2017-02-08 Thread Alexandra Hájková
Some muxers may use the BMP_HEADER Format Data size instead
of the ASF-specific one.

Bug-Id: 1020
---
Use more descriptive variable names.
Upgrate the documentation.
Use better commit message.

 libavformat/asfdec.c  | 12 +++-
 libavformat/avidec.c  |  2 +-
 libavformat/riff.h|  4 ++--
 libavformat/riffdec.c |  6 --
 libavformat/wtv.c |  2 +-
 5 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/libavformat/asfdec.c b/libavformat/asfdec.c
index d602af8..6fe2524 100644
--- a/libavformat/asfdec.c
+++ b/libavformat/asfdec.c
@@ -691,20 +691,22 @@ static int asf_read_properties(AVFormatContext *s, const 
GUIDParseTable *g)
 
 static int parse_video_info(AVIOContext *pb, AVStream *st)
 {
-uint16_t size;
+uint16_t size_asf; // ASF specific Format Data size
+uint32_t size_bmp; // BMP_HEADER specific Format Data size
 unsigned int tag;
 
 st->codecpar->width  = avio_rl32(pb);
 st->codecpar->height = avio_rl32(pb);
 avio_skip(pb, 1); // skip reserved flags
-size = avio_rl16(pb); // size of the Format Data
-tag  = ff_get_bmp_header(pb, st);
+size_asf = avio_rl16(pb);
+tag  = ff_get_bmp_header(pb, st, _bmp);
 st->codecpar->codec_tag = tag;
 st->codecpar->codec_id  = ff_codec_get_id(ff_codec_bmp_tags, tag);
+size_bmp = FFMAX(size_asf, size_bmp);
 
-if (size > BMP_HEADER_SIZE) {
+if (size_bmp > BMP_HEADER_SIZE) {
 int ret;
-st->codecpar->extradata_size  = size - BMP_HEADER_SIZE;
+st->codecpar->extradata_size  = size_bmp - BMP_HEADER_SIZE;
 if (!(st->codecpar->extradata = av_malloc(st->codecpar->extradata_size 
+
AV_INPUT_BUFFER_PADDING_SIZE))) 
{
 st->codecpar->extradata_size = 0;
diff --git a/libavformat/avidec.c b/libavformat/avidec.c
index 0439c9c..61f81e8 100644
--- a/libavformat/avidec.c
+++ b/libavformat/avidec.c
@@ -613,7 +613,7 @@ static int avi_read_header(AVFormatContext *s)
 avio_skip(pb, size);
 break;
 }
-tag1 = ff_get_bmp_header(pb, st);
+tag1 = ff_get_bmp_header(pb, st, NULL);
 
 if (tag1 == MKTAG('D', 'X', 'S', 'B') ||
 tag1 == MKTAG('D', 'X', 'S', 'A')) {
diff --git a/libavformat/riff.h b/libavformat/riff.h
index a45c7f3..e77552b 100644
--- a/libavformat/riff.h
+++ b/libavformat/riff.h
@@ -40,10 +40,10 @@ void ff_end_tag(AVIOContext *pb, int64_t start);
 
 /**
  * Read BITMAPINFOHEADER structure and set AVStream codec width, height and
- * bits_per_encoded_sample fields. Does not read extradata.
+ * bits_per_encoded_sample fields. Writes the size of BMP file to *size. Does 
not read extradata.
  * @return codec tag
  */
-int ff_get_bmp_header(AVIOContext *pb, AVStream *st);
+int ff_get_bmp_header(AVIOContext *pb, AVStream *st, uint32_t *size);
 
 void ff_put_bmp_header(AVIOContext *pb, AVCodecParameters *par, const 
AVCodecTag *tags, int for_asf);
 int ff_put_wav_header(AVFormatContext *s, AVIOContext *pb, AVCodecParameters 
*par);
diff --git a/libavformat/riffdec.c b/libavformat/riffdec.c
index 8124835..d10ea2b 100644
--- a/libavformat/riffdec.c
+++ b/libavformat/riffdec.c
@@ -180,10 +180,12 @@ enum AVCodecID ff_wav_codec_get_id(unsigned int tag, int 
bps)
 return id;
 }
 
-int ff_get_bmp_header(AVIOContext *pb, AVStream *st)
+int ff_get_bmp_header(AVIOContext *pb, AVStream *st, uint32_t *size)
 {
 int tag1;
-avio_rl32(pb); /* size */
+uint32_t size_ = avio_rl32(pb); /* size */
+if (size)
+*size = size_;
 st->codecpar->width  = avio_rl32(pb);
 st->codecpar->height = (int32_t)avio_rl32(pb);
 avio_rl16(pb); /* planes */
diff --git a/libavformat/wtv.c b/libavformat/wtv.c
index 2cab4e5..272b317 100644
--- a/libavformat/wtv.c
+++ b/libavformat/wtv.c
@@ -586,7 +586,7 @@ static int parse_videoinfoheader2(AVFormatContext *s, 
AVStream *st)
 AVIOContext *pb = wtv->pb;
 
 avio_skip(pb, 72);  // picture aspect ratio is unreliable
-ff_get_bmp_header(pb, st);
+ff_get_bmp_header(pb, st, NULL);
 
 return 72 + 40;
 }
-- 
2.1.4

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

Re: [libav-devel] [PATCH 3/5] frame: allow align=0 (meaning automatic) for av_frame_get_buffer()

2017-02-08 Thread Anton Khirnov
Quoting wm4 (2017-02-08 12:08:18)
> On Wed,  8 Feb 2017 11:50:58 +0100
> Anton Khirnov  wrote:
> 
> > This will avoid every caller from hardcoding some specific alignment,
> > which may break in the future with new instruction sets.
> > ---
> 
> >   * @param frame frame in which to store the new buffers.
> > - * @param align required buffer size alignment
> > + * @param align Required buffer size alignment. If equal to 0, alignment 
> > will be
> > + *  chosen automatically for the current CPU. It is highly
> > + *  recommended to pass 0 here unless you know what you are 
> > doing.
> >   *
> >   * @return 0 on success, a negative AVERROR on error.
> >   */
> 
> This implicitly changes the meaning of "0" from no alignment
> (equivalent to 1) to some random alignment. I don't think that's very
> safe. Why not make -1 the magic value that enables automatic alignment?

0 is not "no alignment", 0 is invalid for video and I think will do
something random right now. For audio, 0 already means automatic. So I'm
not changing the behaviour of any valid code.

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

Re: [libav-devel] [PATCH 3/5] frame: allow align=0 (meaning automatic) for av_frame_get_buffer()

2017-02-08 Thread wm4
On Wed,  8 Feb 2017 11:50:58 +0100
Anton Khirnov  wrote:

> This will avoid every caller from hardcoding some specific alignment,
> which may break in the future with new instruction sets.
> ---

>   * @param frame frame in which to store the new buffers.
> - * @param align required buffer size alignment
> + * @param align Required buffer size alignment. If equal to 0, alignment 
> will be
> + *  chosen automatically for the current CPU. It is highly
> + *  recommended to pass 0 here unless you know what you are 
> doing.
>   *
>   * @return 0 on success, a negative AVERROR on error.
>   */

This implicitly changes the meaning of "0" from no alignment
(equivalent to 1) to some random alignment. I don't think that's very
safe. Why not make -1 the magic value that enables automatic alignment?
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

[libav-devel] [PATCH 1/5] cpu: add a function for querying maximum required data alignment

2017-02-08 Thread Anton Khirnov
---
 doc/APIchanges  |  3 +++
 libavutil/cpu.c | 13 +
 libavutil/cpu.h | 13 +
 libavutil/version.h |  2 +-
 4 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/doc/APIchanges b/doc/APIchanges
index c161618..acd1536 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -13,6 +13,9 @@ libavutil: 2015-08-28
 
 API changes, most recent first:
 
+2017-02-xx - xxx - lavu 55.31.0 - cpu.h
+  Add av_cpu_max_align() for querying maximum required data alignment.
+
 2017-02-01 - xxx - lavc - avcodec.h
   Deprecate AVCodecContext.refcounted_frames. This was useful for deprecated
   API only (avcodec_decode_video2/avcodec_decode_audio4). The new decode APIs
diff --git a/libavutil/cpu.c b/libavutil/cpu.c
index 0109c9e..5aef6af 100644
--- a/libavutil/cpu.c
+++ b/libavutil/cpu.c
@@ -16,6 +16,7 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */
 
+#include 
 #include 
 #include 
 
@@ -180,3 +181,15 @@ int av_cpu_count(void)
 
 return nb_cpus;
 }
+
+size_t av_cpu_max_align(void)
+{
+int flags = av_get_cpu_flags();
+
+if (flags & AV_CPU_FLAG_AVX)
+return 32;
+if (flags & (AV_CPU_FLAG_ALTIVEC | AV_CPU_FLAG_SSE | AV_CPU_FLAG_NEON))
+return 16;
+
+return 8;
+}
diff --git a/libavutil/cpu.h b/libavutil/cpu.h
index c205ee1..5790f9e 100644
--- a/libavutil/cpu.h
+++ b/libavutil/cpu.h
@@ -21,6 +21,8 @@
 #ifndef AVUTIL_CPU_H
 #define AVUTIL_CPU_H
 
+#include 
+
 #include "version.h"
 
 #define AV_CPU_FLAG_FORCE0x8000 /* force usage of selected flags (OR) 
*/
@@ -88,4 +90,15 @@ int av_parse_cpu_flags(const char *s);
  */
 int av_cpu_count(void);
 
+/**
+ * Get the maximum data alignment that may be required by Libav.
+ *
+ * Note that this is affected by the build configuration and the CPU flags 
mask,
+ * so e.g. if the CPU supports AVX, but libavutil has been built with
+ * --disable-avx or the AV_CPU_FLAG_AVX has been disabled through
+ *  av_set_cpu_flags_mask(), then this function will behave as if AVX is not
+ *  present.
+ */
+size_t av_cpu_max_align(void);
+
 #endif /* AVUTIL_CPU_H */
diff --git a/libavutil/version.h b/libavutil/version.h
index 7856a0a..0fcd19a 100644
--- a/libavutil/version.h
+++ b/libavutil/version.h
@@ -54,7 +54,7 @@
  */
 
 #define LIBAVUTIL_VERSION_MAJOR 55
-#define LIBAVUTIL_VERSION_MINOR 30
+#define LIBAVUTIL_VERSION_MINOR 31
 #define LIBAVUTIL_VERSION_MICRO  0
 
 #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
-- 
2.0.0

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

[libav-devel] [PATCH 5/5] vf_interlace/x86: process only one block per iteration

2017-02-08 Thread Anton Khirnov
Nothing guarantees that the line is large enough for two blocks, so
current code may result in invalid memory access. This change makes the
function ~5% slower, but since this is a highly obscure filter that
nobody should use, its performance is not very important.
---
 libavfilter/x86/vf_interlace.asm | 23 ---
 1 file changed, 8 insertions(+), 15 deletions(-)

diff --git a/libavfilter/x86/vf_interlace.asm b/libavfilter/x86/vf_interlace.asm
index 7302314..7cff3c0 100644
--- a/libavfilter/x86/vf_interlace.asm
+++ b/libavfilter/x86/vf_interlace.asm
@@ -28,32 +28,25 @@ SECTION_RODATA
 SECTION .text
 
 %macro LOWPASS_LINE 0
-cglobal lowpass_line, 5, 5, 7, dst, linesize, src, src_above, src_below
+cglobal lowpass_line, 5, 5, 3, dst, linesize, src, src_above, src_below
 add dstq,   linesizeq
 add srcq,   linesizeq
 add src_aboveq, linesizeq
 add src_belowq, linesizeq
 neg linesizeq
 
-pcmpeqb m6, m6
+pcmpeqb m2, m2
 
 .loop:
 mova  m0, [src_aboveq + linesizeq]
-mova  m1, [src_aboveq + linesizeq + mmsize]
 pavgb m0, [src_belowq + linesizeq]
-pavgb m1, [src_belowq + linesizeq + mmsize]
-pxor m0, m6
-pxor m1, m6
-pxor m2, m6, [srcq + linesizeq]
-pxor m3, m6, [srcq + linesizeq + mmsize]
-pavgb m0, m2
-pavgb m1, m3
-pxor m0, m6
-pxor m1, m6
-mova [dstq + linesizeq],  m0
-mova [dstq + linesizeq + mmsize], m1
+pxor  m0, m2
+pxor  m1, m2, [srcq + linesizeq]
+pavgb m0, m1
+pxor  m0, m2
+mova [dstq + linesizeq], m0
 
-add linesizeq, 2 * mmsize
+add linesizeq, mmsize
 jl .loop
 REP_RET
 %endmacro
-- 
2.0.0

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

[libav-devel] [PATCH 2/5] lavc: use av_cpu_max_align() instead of hardcoding alignment requirements

2017-02-08 Thread Anton Khirnov
---
 libavcodec/utils.c | 11 ++-
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index 2978109..06a5784 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -179,17 +179,10 @@ int ff_side_data_update_matrix_encoding(AVFrame *frame,
 return 0;
 }
 
-#if HAVE_SIMD_ALIGN_32
-#   define STRIDE_ALIGN 32
-#elif HAVE_SIMD_ALIGN_16
-#   define STRIDE_ALIGN 16
-#else
-#   define STRIDE_ALIGN 8
-#endif
-
 void avcodec_align_dimensions2(AVCodecContext *s, int *width, int *height,
int linesize_align[AV_NUM_DATA_POINTERS])
 {
+size_t max_align = av_cpu_max_align();
 int i;
 int w_align = 1;
 int h_align = 1;
@@ -282,7 +275,7 @@ void avcodec_align_dimensions2(AVCodecContext *s, int 
*width, int *height,
 *height += 2;
 
 for (i = 0; i < 4; i++)
-linesize_align[i] = STRIDE_ALIGN;
+linesize_align[i] = max_align;
 }
 
 void avcodec_align_dimensions(AVCodecContext *s, int *width, int *height)
-- 
2.0.0

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

[libav-devel] [PATCH 4/5] vf_interlace/x86: use symbolic names for better readability

2017-02-08 Thread Anton Khirnov
---
 libavfilter/x86/vf_interlace.asm | 30 +++---
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/libavfilter/x86/vf_interlace.asm b/libavfilter/x86/vf_interlace.asm
index f234421..7302314 100644
--- a/libavfilter/x86/vf_interlace.asm
+++ b/libavfilter/x86/vf_interlace.asm
@@ -28,32 +28,32 @@ SECTION_RODATA
 SECTION .text
 
 %macro LOWPASS_LINE 0
-cglobal lowpass_line, 5, 5, 7
-add r0, r1
-add r2, r1
-add r3, r1
-add r4, r1
-neg r1
+cglobal lowpass_line, 5, 5, 7, dst, linesize, src, src_above, src_below
+add dstq,   linesizeq
+add srcq,   linesizeq
+add src_aboveq, linesizeq
+add src_belowq, linesizeq
+neg linesizeq
 
 pcmpeqb m6, m6
 
 .loop:
-mova m0, [r3+r1]
-mova m1, [r3+r1+mmsize]
-pavgb m0, [r4+r1]
-pavgb m1, [r4+r1+mmsize]
+mova  m0, [src_aboveq + linesizeq]
+mova  m1, [src_aboveq + linesizeq + mmsize]
+pavgb m0, [src_belowq + linesizeq]
+pavgb m1, [src_belowq + linesizeq + mmsize]
 pxor m0, m6
 pxor m1, m6
-pxor m2, m6, [r2+r1]
-pxor m3, m6, [r2+r1+mmsize]
+pxor m2, m6, [srcq + linesizeq]
+pxor m3, m6, [srcq + linesizeq + mmsize]
 pavgb m0, m2
 pavgb m1, m3
 pxor m0, m6
 pxor m1, m6
-mova [r0+r1], m0
-mova [r0+r1+mmsize], m1
+mova [dstq + linesizeq],  m0
+mova [dstq + linesizeq + mmsize], m1
 
-add r1, 2*mmsize
+add linesizeq, 2 * mmsize
 jl .loop
 REP_RET
 %endmacro
-- 
2.0.0

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

[libav-devel] [PATCH 3/5] frame: allow align=0 (meaning automatic) for av_frame_get_buffer()

2017-02-08 Thread Anton Khirnov
This will avoid every caller from hardcoding some specific alignment,
which may break in the future with new instruction sets.
---
 doc/APIchanges  | 4 
 libavutil/frame.c   | 4 
 libavutil/frame.h   | 4 +++-
 libavutil/version.h | 2 +-
 4 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/doc/APIchanges b/doc/APIchanges
index acd1536..fd751f3 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -13,6 +13,10 @@ libavutil: 2015-08-28
 
 API changes, most recent first:
 
+2017-02-xx - xxx - lavu 55.31.1 - frame.h
+  Allow passing the value of 0 (meaning "automatic") as the required alignment
+  to av_frame_get_buffer().
+
 2017-02-xx - xxx - lavu 55.31.0 - cpu.h
   Add av_cpu_max_align() for querying maximum required data alignment.
 
diff --git a/libavutil/frame.c b/libavutil/frame.c
index aafaa57..aa5820c 100644
--- a/libavutil/frame.c
+++ b/libavutil/frame.c
@@ -19,6 +19,7 @@
 #include "channel_layout.h"
 #include "buffer.h"
 #include "common.h"
+#include "cpu.h"
 #include "dict.h"
 #include "frame.h"
 #include "imgutils.h"
@@ -103,6 +104,9 @@ static int get_video_buffer(AVFrame *frame, int align)
 if (ret < 0)
 return ret;
 
+if (align <= 0)
+align = av_cpu_max_align();
+
 for (i = 0; i < 4 && frame->linesize[i]; i++)
 frame->linesize[i] = FFALIGN(frame->linesize[i], align);
 }
diff --git a/libavutil/frame.h b/libavutil/frame.h
index c718f7b..4f63fb0 100644
--- a/libavutil/frame.h
+++ b/libavutil/frame.h
@@ -475,7 +475,9 @@ void av_frame_move_ref(AVFrame *dst, AVFrame *src);
  *   cases.
  *
  * @param frame frame in which to store the new buffers.
- * @param align required buffer size alignment
+ * @param align Required buffer size alignment. If equal to 0, alignment will 
be
+ *  chosen automatically for the current CPU. It is highly
+ *  recommended to pass 0 here unless you know what you are doing.
  *
  * @return 0 on success, a negative AVERROR on error.
  */
diff --git a/libavutil/version.h b/libavutil/version.h
index 0fcd19a..0768f9f 100644
--- a/libavutil/version.h
+++ b/libavutil/version.h
@@ -55,7 +55,7 @@
 
 #define LIBAVUTIL_VERSION_MAJOR 55
 #define LIBAVUTIL_VERSION_MINOR 31
-#define LIBAVUTIL_VERSION_MICRO  0
+#define LIBAVUTIL_VERSION_MICRO  1
 
 #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
LIBAVUTIL_VERSION_MINOR, \
-- 
2.0.0

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

Re: [libav-devel] [libav-commits] configure: Rework dependency handling for conflicting components

2017-02-08 Thread Martin Storsjö

On Wed, 8 Feb 2017, Diego Biurrun  wrote:


Module: libav
Branch: master
Commit: dad7a9c7c0ae8ebc56f2e3a24e6fa4da5c2cd491

Author:Diego Biurrun 
Committer: Diego Biurrun 
Date:  Fri Jan 20 17:17:16 2017 +0100

configure: Rework dependency handling for conflicting components

This makes the feature more visible and obvious.

---

configure | 22 +-
1 file changed, 13 insertions(+), 9 deletions(-)


This broke building on OS X, where the avfoundation indevice no longer 
gets the necessary objc libs linked in:


https://fate.libav.org/x86_64-apple-darwin-clang-8.0/20170208095533

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

Re: [libav-devel] [PATCH] asfdec: use the BMP_HEADER specific Format Data size instead of

2017-02-08 Thread Martin Storsjö

On Wed, 8 Feb 2017, Anton Khirnov wrote:


Quoting Diego Biurrun (2017-02-07 17:39:38)

On Tue, Feb 07, 2017 at 02:15:55PM +0100, Alexandra Hájková wrote:
> the ASF specific Format Data size. Fixes video decoding problem
> part of the bug 1020.

See the other review for log message hints.

> --- a/libavformat/asfdec.c
> +++ b/libavformat/asfdec.c
> @@ -691,16 +691,18 @@ static int asf_read_properties(AVFormatContext *s, 
const GUIDParseTable *g)
> 
>  static int parse_video_info(AVIOContext *pb, AVStream *st)

>  {
> +uint16_t size_;
> +uint32_t size;
>  unsigned int tag;

Please use descriptive variable names.

> --- a/libavformat/riff.h
> +++ b/libavformat/riff.h
> @@ -43,7 +43,7 @@ void ff_end_tag(AVIOContext *pb, int64_t start);
>   * bits_per_encoded_sample fields. Does not read extradata.
>   * @return codec tag
>   */
> -int ff_get_bmp_header(AVIOContext *pb, AVStream *st);
> +int ff_get_bmp_header(AVIOContext *pb, AVStream *st, uint32_t *size);

The documentation is now incomplete.

Why uint32_t? Sizes should have size_t as type.


Your size_t obsession is getting out of control. It makes sense for some
places, but not all of them. As is rather obvious from the patch, the
size is stored as uint32 in the bitstream, therefore it makes sense to
store it as uint32 in code.


To elaborate on this: If you make such fields a size_t, you end up parsing 
files differently depending on whether you're using a 32 or 64 bit build.


For such values; if they are supposed to be over 32 bits, use an uint64_t 
and it will work consistently and correctly on both. If it isn't supposed 
to be that, use an uint32_t.


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

Re: [libav-devel] [PATCH] asfdec: use the BMP_HEADER specific Format Data size instead of

2017-02-08 Thread Anton Khirnov
Quoting Diego Biurrun (2017-02-07 17:39:38)
> On Tue, Feb 07, 2017 at 02:15:55PM +0100, Alexandra Hájková wrote:
> > the ASF specific Format Data size. Fixes video decoding problem
> > part of the bug 1020.
> 
> See the other review for log message hints.
> 
> > --- a/libavformat/asfdec.c
> > +++ b/libavformat/asfdec.c
> > @@ -691,16 +691,18 @@ static int asf_read_properties(AVFormatContext *s, 
> > const GUIDParseTable *g)
> >  
> >  static int parse_video_info(AVIOContext *pb, AVStream *st)
> >  {
> > +uint16_t size_;
> > +uint32_t size;
> >  unsigned int tag;
> 
> Please use descriptive variable names.
> 
> > --- a/libavformat/riff.h
> > +++ b/libavformat/riff.h
> > @@ -43,7 +43,7 @@ void ff_end_tag(AVIOContext *pb, int64_t start);
> >   * bits_per_encoded_sample fields. Does not read extradata.
> >   * @return codec tag
> >   */
> > -int ff_get_bmp_header(AVIOContext *pb, AVStream *st);
> > +int ff_get_bmp_header(AVIOContext *pb, AVStream *st, uint32_t *size);
> 
> The documentation is now incomplete.
> 
> Why uint32_t? Sizes should have size_t as type.

Your size_t obsession is getting out of control. It makes sense for some
places, but not all of them. As is rather obvious from the patch, the
size is stored as uint32 in the bitstream, therefore it makes sense to
store it as uint32 in code.

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