Re: [libav-devel] [PATCH 1/2] hls: Tell the actual segment being opened in verbose mode

[Vittorio Giovara]

On Thu, Feb 15, 2018 at 3:20 PM, Luca Barbato  wrote:

> ---
>  libavformat/hls.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/libavformat/hls.c b/libavformat/hls.c
> index c9da4e30ec..5aa633269f 100644
> --- a/libavformat/hls.c
> +++ b/libavformat/hls.c
> @@ -362,6 +362,7 @@ static int open_input(struct variant *var)
>  {
>  HLSContext *c = var->parent->priv_data;
>  struct segment *seg = var->segments[var->cur_seq_no -
> var->start_seq_no];
> +av_log(var->parent, AV_LOG_VERBOSE, "Opening %s\n", seg->url);
>  if (seg->key_type == KEY_NONE) {
>  return open_url(var->parent, >input, seg->url, c->avio_opts);
>  } else if (seg->key_type == KEY_AES_128) {
> --
>
>
seems fine
-- 
Vittorio
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH 2/2] hls: Use the correct logging context

[Vittorio Giovara]

On Thu, Feb 15, 2018 at 3:20 PM, Luca Barbato  wrote:

> ---
>  libavformat/hls.c | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/libavformat/hls.c b/libavformat/hls.c
> index 5aa633269f..6876b8411e 100644
> --- a/libavformat/hls.c
> +++ b/libavformat/hls.c
> @@ -374,12 +374,12 @@ static int open_input(struct variant *var)
>  if (open_url(var->parent, , seg->key, c->avio_opts) == 0) {
>  ret = avio_read(pb, var->key, sizeof(var->key));
>  if (ret != sizeof(var->key)) {
> -av_log(NULL, AV_LOG_ERROR, "Unable to read key file
> %s\n",
> +av_log(var->parent, AV_LOG_ERROR, "Unable to read key
> file %s\n",
> seg->key);
>  }
>  ff_format_io_close(var->parent, );
>  } else {
> -av_log(NULL, AV_LOG_ERROR, "Unable to open key file %s\n",
> +av_log(var->parent, AV_LOG_ERROR, "Unable to open key
> file %s\n",
> seg->key);
>  }
>  av_strlcpy(var->key_url, seg->key, sizeof(var->key_url));
> @@ -428,7 +428,7 @@ reload:
>  reload_interval = v->target_duration / 2;
>  }
>  if (v->cur_seq_no < v->start_seq_no) {
> -av_log(NULL, AV_LOG_WARNING,
> +av_log(v->parent, AV_LOG_WARNING,
> "skipping %d segments ahead, expired from playlists\n",
> v->start_seq_no - v->cur_seq_no);
>  v->cur_seq_no = v->start_seq_no;
> @@ -521,7 +521,7 @@ static int hls_read_header(AVFormatContext *s)
>  goto fail;
>
>  if (c->n_variants == 0) {
> -av_log(NULL, AV_LOG_WARNING, "Empty playlist\n");
> +av_log(s, AV_LOG_WARNING, "Empty playlist\n");
>  ret = AVERROR_EOF;
>  goto fail;
>  }
> @@ -536,7 +536,7 @@ static int hls_read_header(AVFormatContext *s)
>  }
>
>  if (c->variants[0]->n_segments == 0) {
> -av_log(NULL, AV_LOG_WARNING, "Empty playlist\n");
> +av_log(s, AV_LOG_WARNING, "Empty playlist\n");
>  ret = AVERROR_EOF;
>  goto fail;
>  }
> --
>

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

[libav-devel] [PATCH 1/2] hls: Tell the actual segment being opened in verbose mode

[Luca Barbato]

---
 libavformat/hls.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libavformat/hls.c b/libavformat/hls.c
index c9da4e30ec..5aa633269f 100644
--- a/libavformat/hls.c
+++ b/libavformat/hls.c
@@ -362,6 +362,7 @@ static int open_input(struct variant *var)
 {
 HLSContext *c = var->parent->priv_data;
 struct segment *seg = var->segments[var->cur_seq_no - var->start_seq_no];
+av_log(var->parent, AV_LOG_VERBOSE, "Opening %s\n", seg->url);
 if (seg->key_type == KEY_NONE) {
 return open_url(var->parent, >input, seg->url, c->avio_opts);
 } else if (seg->key_type == KEY_AES_128) {
-- 
2.12.2

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

[libav-devel] [PATCH 2/2] hls: Use the correct logging context

[Luca Barbato]

---
 libavformat/hls.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/libavformat/hls.c b/libavformat/hls.c
index 5aa633269f..6876b8411e 100644
--- a/libavformat/hls.c
+++ b/libavformat/hls.c
@@ -374,12 +374,12 @@ static int open_input(struct variant *var)
 if (open_url(var->parent, , seg->key, c->avio_opts) == 0) {
 ret = avio_read(pb, var->key, sizeof(var->key));
 if (ret != sizeof(var->key)) {
-av_log(NULL, AV_LOG_ERROR, "Unable to read key file %s\n",
+av_log(var->parent, AV_LOG_ERROR, "Unable to read key file 
%s\n",
seg->key);
 }
 ff_format_io_close(var->parent, );
 } else {
-av_log(NULL, AV_LOG_ERROR, "Unable to open key file %s\n",
+av_log(var->parent, AV_LOG_ERROR, "Unable to open key file 
%s\n",
seg->key);
 }
 av_strlcpy(var->key_url, seg->key, sizeof(var->key_url));
@@ -428,7 +428,7 @@ reload:
 reload_interval = v->target_duration / 2;
 }
 if (v->cur_seq_no < v->start_seq_no) {
-av_log(NULL, AV_LOG_WARNING,
+av_log(v->parent, AV_LOG_WARNING,
"skipping %d segments ahead, expired from playlists\n",
v->start_seq_no - v->cur_seq_no);
 v->cur_seq_no = v->start_seq_no;
@@ -521,7 +521,7 @@ static int hls_read_header(AVFormatContext *s)
 goto fail;
 
 if (c->n_variants == 0) {
-av_log(NULL, AV_LOG_WARNING, "Empty playlist\n");
+av_log(s, AV_LOG_WARNING, "Empty playlist\n");
 ret = AVERROR_EOF;
 goto fail;
 }
@@ -536,7 +536,7 @@ static int hls_read_header(AVFormatContext *s)
 }
 
 if (c->variants[0]->n_segments == 0) {
-av_log(NULL, AV_LOG_WARNING, "Empty playlist\n");
+av_log(s, AV_LOG_WARNING, "Empty playlist\n");
 ret = AVERROR_EOF;
 goto fail;
 }
-- 
2.12.2

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

Re: [libav-devel] [PATCH] fate: Provide "unknown" configuration values as fallback

[Martin Storsjö]

On Thu, 15 Feb 2018, Diego Biurrun wrote:


If configure fails before config.fate is generated, these values will be
used and thus the generated report will be accepted by the FATE server.
---

This will address the issues I have had with Martin's fate instances.

tests/fate.sh | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tests/fate.sh b/tests/fate.sh
index c93e20a464..03a3728006 100755
--- a/tests/fate.sh
+++ b/tests/fate.sh
@@ -84,7 +84,10 @@ clean(){
report(){
date=$(date -u +%Y%m%d%H%M%S)
echo "fate:1:${date}:${slot}:${version}:$1:$2:${branch}:${comment}" >report
-cat ${build}/avbuild/config.fate ${build}/tests/data/fate/*.rep >> report 
2> /dev/null
+test -e ${build}/avbuild/config.fate &&
+cat ${build}/avbuild/config.fate >> report 2> /dev/null ||
+echo config:unknown:unknown:unknown:unknown:unknown:unknown >> report 
2> /dev/null
+cat ${build}/tests/data/fate/*.rep >> report 2> /dev/null
test -n "$fate_recv" && $tar report *.log | gzip | $fate_recv
}

--
2.11.0


Ok with me.

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

[libav-devel] [PATCH] fate: Provide "unknown" configuration values as fallback

[Diego Biurrun]

If configure fails before config.fate is generated, these values will be
used and thus the generated report will be accepted by the FATE server.
---

This will address the issues I have had with Martin's fate instances.

 tests/fate.sh | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tests/fate.sh b/tests/fate.sh
index c93e20a464..03a3728006 100755
--- a/tests/fate.sh
+++ b/tests/fate.sh
@@ -84,7 +84,10 @@ clean(){
 report(){
 date=$(date -u +%Y%m%d%H%M%S)
 echo "fate:1:${date}:${slot}:${version}:$1:$2:${branch}:${comment}" >report
-cat ${build}/avbuild/config.fate ${build}/tests/data/fate/*.rep >> report 
2> /dev/null
+test -e ${build}/avbuild/config.fate &&
+cat ${build}/avbuild/config.fate >> report 2> /dev/null ||
+echo config:unknown:unknown:unknown:unknown:unknown:unknown >> report 
2> /dev/null
+cat ${build}/tests/data/fate/*.rep >> report 2> /dev/null
 test -n "$fate_recv" && $tar report *.log | gzip | $fate_recv
 }
 
-- 
2.11.0

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

Re: [libav-devel] [PATCH 5/6] vf_crop: Reindent after previous commit

[Mark Thompson]

On 15/02/18 09:29, Diego Biurrun wrote:
> On Wed, Feb 14, 2018 at 10:54:30PM +, Mark Thompson wrote:
>> ---
>>  libavfilter/vf_crop.c | 30 +++---
>>  1 file changed, 15 insertions(+), 15 deletions(-)
> 
> Just squash it with 4/6 IMO.

Ok, sure.  (The diff is easier to read in this form, at least.)

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

Re: [libav-devel] [PATCH 1/6] lavfi: VAAPI VPP common infrastructure.

[Mark Thompson]

On 15/02/18 09:27, Diego Biurrun wrote:
> On Wed, Feb 14, 2018 at 10:54:26PM +, Mark Thompson wrote:
>> --- /dev/null
>> +++ b/libavfilter/vaapi_vpp.c
>> @@ -0,0 +1,370 @@
>> +
>> +#include 
>> +
>> +#include "config.h"
> 
> This does not appear to use config.h.

...
+if (HAVE_VAAPI_1 || ctx->hwctx->driver_quirks &
...

>> +#include "libavutil/avassert.h"
>> +#include "libavutil/mem.h"
>> +#include "libavutil/pixdesc.h"
>> +#include "formats.h"
>> +#include "internal.h"
>> +#include "vaapi_vpp.h"
> 
> nit: empty line between header groups
> 
>> +int ff_vaapi_vpp_config_input(AVFilterLink *inlink)
>> +{
>> +ctx->input_frames_ref = av_buffer_ref(inlink->hw_frames_ctx);
>> +if (!ctx->input_frames_ref) {
>> +av_log(avctx, AV_LOG_ERROR, "A input frames reference create "
>> +   "failed.\n");
> 
> That sentence sounds ungrammatical to me.

It's ENOMEM, I've just removed the message.

>> +ctx->input_frames = (AVHWFramesContext*)ctx->input_frames_ref->data;
> 
> nit: space before *
> 
>> +ctx->device_ref = av_buffer_ref(ctx->input_frames->device_ref);
>> +if (!ctx->device_ref) {
>> +av_log(avctx, AV_LOG_ERROR, "A device reference create "
>> +   "failed.\n");
> 
> see above

Likewise.

>> +ctx->hwctx = ((AVHWDeviceContext*)ctx->device_ref->data)->hwctx;
> 
> see above
> 
>> +int ff_vaapi_vpp_colour_standard(enum AVColorSpace av_cs)
>> +{
>> +switch(av_cs) {
> 
> switch (
> 
>> +#define CS(av, va) case AVCOL_SPC_ ## av: return VAProcColorStandard ## va;
>> +CS(BT709, BT709);
>> +CS(BT470BG,   BT601);
>> +CS(SMPTE170M, SMPTE170M);
>> +CS(SMPTE240M, SMPTE240M);
>> +#undef CS
>> +default:
>> +return VAProcColorStandardNone;
>> +}
>> +}
> 
> I don't think the #define eases readability, on the contrary.

Disagree?  In any case, this function isn't really correct because it doesn't 
make use of all of the colour information, and there are also many more cases 
in libva 2.0.  I'll send a fix for that at some point, but for now it's just 
factorising out the code which was previously in individual filters.

>> +int ff_vaapi_vpp_render_picture(AVFilterContext *avctx,
>> +VAProcPipelineParameterBuffer *params,
>> +VASurfaceID output_surface)
>> +{
>> +int err = 0;
>> +VAAPIVPPContext *ctx   = avctx->priv;
> 
> odd extra space
> 
>> +void ff_vaapi_vpp_ctx_init(AVFilterContext *avctx)
>> +{
>> +int i;
>> +VAAPIVPPContext *ctx   = avctx->priv;
> 
> same
> 
>> +void ff_vaapi_vpp_ctx_uninit(AVFilterContext *avctx)
>> +{
>> +VAAPIVPPContext *ctx   = avctx->priv;
> 
> same

Sure, all removed.

>> --- /dev/null
>> +++ b/libavfilter/vaapi_vpp.h
>> @@ -0,0 +1,79 @@
>> +#ifndef AVFILTER_VAAPI_VPP_H
>> +#define AVFILTER_VAAPI_VPP_H
>> +
>> +#include 
>> +#include 
> 
> I think this needs to go in SKIPHEADERS.

Yes.

Thanks,

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

Re: [libav-devel] [PATCH 1/6] lavfi: VAAPI VPP common infrastructure.

[Diego Biurrun]

On Thu, Feb 15, 2018 at 10:27:15AM +0100, Diego Biurrun wrote:
> On Wed, Feb 14, 2018 at 10:54:26PM +, Mark Thompson wrote:
> > --- /dev/null
> > +++ b/libavfilter/vaapi_vpp.c
> > @@ -0,0 +1,370 @@
> > +int ff_vaapi_vpp_colour_standard(enum AVColorSpace av_cs)
> > +{
> > +switch(av_cs) {
> > +#define CS(av, va) case AVCOL_SPC_ ## av: return VAProcColorStandard ## va;
> > +CS(BT709, BT709);
> > +CS(BT470BG,   BT601);
> > +CS(SMPTE170M, SMPTE170M);
> > +CS(SMPTE240M, SMPTE240M);
> > +#undef CS
> > +default:
> > +return VAProcColorStandardNone;
> > +}
> > +}
> 
> I don't think the #define eases readability, on the contrary.

I see this is copy+pasted from other files, oh well ..

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

Re: [libav-devel] [PATCH 5/6] vf_crop: Reindent after previous commit

[Diego Biurrun]

On Wed, Feb 14, 2018 at 10:54:30PM +, Mark Thompson wrote:
> ---
>  libavfilter/vf_crop.c | 30 +++---
>  1 file changed, 15 insertions(+), 15 deletions(-)

Just squash it with 4/6 IMO.

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

Re: [libav-devel] [PATCH 1/6] lavfi: VAAPI VPP common infrastructure.

[Diego Biurrun]

On Wed, Feb 14, 2018 at 10:54:26PM +, Mark Thompson wrote:
> --- /dev/null
> +++ b/libavfilter/vaapi_vpp.c
> @@ -0,0 +1,370 @@
> +
> +#include 
> +
> +#include "config.h"

This does not appear to use config.h.

> +#include "libavutil/avassert.h"
> +#include "libavutil/mem.h"
> +#include "libavutil/pixdesc.h"
> +#include "formats.h"
> +#include "internal.h"
> +#include "vaapi_vpp.h"

nit: empty line between header groups

> +int ff_vaapi_vpp_config_input(AVFilterLink *inlink)
> +{
> +ctx->input_frames_ref = av_buffer_ref(inlink->hw_frames_ctx);
> +if (!ctx->input_frames_ref) {
> +av_log(avctx, AV_LOG_ERROR, "A input frames reference create "
> +   "failed.\n");

That sentence sounds ungrammatical to me.

> +ctx->input_frames = (AVHWFramesContext*)ctx->input_frames_ref->data;

nit: space before *

> +ctx->device_ref = av_buffer_ref(ctx->input_frames->device_ref);
> +if (!ctx->device_ref) {
> +av_log(avctx, AV_LOG_ERROR, "A device reference create "
> +   "failed.\n");

see above

> +ctx->hwctx = ((AVHWDeviceContext*)ctx->device_ref->data)->hwctx;

see above

> +int ff_vaapi_vpp_colour_standard(enum AVColorSpace av_cs)
> +{
> +switch(av_cs) {

switch (

> +#define CS(av, va) case AVCOL_SPC_ ## av: return VAProcColorStandard ## va;
> +CS(BT709, BT709);
> +CS(BT470BG,   BT601);
> +CS(SMPTE170M, SMPTE170M);
> +CS(SMPTE240M, SMPTE240M);
> +#undef CS
> +default:
> +return VAProcColorStandardNone;
> +}
> +}

I don't think the #define eases readability, on the contrary.

> +int ff_vaapi_vpp_render_picture(AVFilterContext *avctx,
> +VAProcPipelineParameterBuffer *params,
> +VASurfaceID output_surface)
> +{
> +int err = 0;
> +VAAPIVPPContext *ctx   = avctx->priv;

odd extra space

> +void ff_vaapi_vpp_ctx_init(AVFilterContext *avctx)
> +{
> +int i;
> +VAAPIVPPContext *ctx   = avctx->priv;

same

> +void ff_vaapi_vpp_ctx_uninit(AVFilterContext *avctx)
> +{
> +VAAPIVPPContext *ctx   = avctx->priv;

same

> --- /dev/null
> +++ b/libavfilter/vaapi_vpp.h
> @@ -0,0 +1,79 @@
> +#ifndef AVFILTER_VAAPI_VPP_H
> +#define AVFILTER_VAAPI_VPP_H
> +
> +#include 
> +#include 

I think this needs to go in SKIPHEADERS.

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