date:20181029

Re: [FFmpeg-devel] [PATCH V2] libavfilter/vaapi: enable vaapi rotation feature via call Intel iHD driver

2018-10-29 Thread Zhou, Zachary



> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf Of
> myp...@gmail.com
> Sent: Monday, October 29, 2018 9:58 AM
> To: FFmpeg development discussions and patches 
> Subject: Re: [FFmpeg-devel] [PATCH V2] libavfilter/vaapi: enable vaapi
> rotation feature via call Intel iHD driver
> 
> On Fri, Oct 26, 2018 at 11:26 PM Paul B Mahol  wrote:
> >
> > On 10/26/18, Zhou, Zachary  wrote:
> > >
> > >
> > >> -Original Message-
> > >> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On
> > >> Behalf Of Rostislav Pehlivanov
> > >> Sent: Friday, October 26, 2018 9:02 PM
> > >> To: FFmpeg development discussions and patches
> > >> 
> > >> Subject: Re: [FFmpeg-devel] [PATCH V2] libavfilter/vaapi: enable
> > >> vaapi rotation feature via call Intel iHD driver
> > >>
> > >> On Fri, 26 Oct 2018 at 04:10, Zhou, Zachary
> > >> 
> > >> wrote:
> > >>
> > >> >
> > >> >
> > >> > > -Original Message-
> > >> > > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On
> > >> > > Behalf Of sean darcy
> > >> > > Sent: Thursday, October 25, 2018 11:51 PM
> > >> > > To: ffmpeg-devel@ffmpeg.org
> > >> > > Subject: Re: [FFmpeg-devel] [PATCH V2] libavfilter/vaapi:
> > >> > > enable vaapi rotation feature via call Intel iHD driver
> > >> > >
> > >> > > On 10/25/18 2:52 AM, Zachary Zhou wrote:
> > >> > > > It supports clockwise rotation by 0/90/180/270 degrees
> > >> > > > defined in va/va_vpp.h, tested following command line on SKL
> > >> > > > platform
> > >> > > >
> > >> > > > dir=0 for 0   degree
> > >> > > > dir=1 for 90  degree
> > >> > > > dir=2 for 180 degree
> > >> > > > dir=3 for 270 degree
> > >> > > >
> > >> > > > ffmpeg -hwaccel vaapi -vaapi_device /dev/dri/renderD128
> > >> > > > -hwaccel_output_format vaapi -i input.264 -vf
> > >> > > > "rotation_vaapi=dir=1"
> > >> > > > -c:v h264_vaapi output.h264
> > >> > > >
> > >> > > > Signed-off-by: Zachary Zhou 
> > >> > > > ---
> > >> > > >   configure |   3 +
> > >> > > >   libavfilter/Makefile  |   1 +
> > >> > > >   libavfilter/allfilters.c  |   1 +
> > >> > > >   libavfilter/vaapi_vpp.h   |   1 +
> > >> > > >   libavfilter/vf_rotate_vaapi.c | 252
> > >> > ++
> > >> > > >   5 files changed, 258 insertions(+)
> > >> > > >   create mode 100644 libavfilter/vf_rotate_vaapi.c
> > >> > > >
> > >> > > > diff --git a/configure b/configure index
> > >> > > > 85d5dd5962..33aced3d78
> > >> > > > 100755
> > >> > > > --- a/configure
> > >> > > > +++ b/configure
> > >> > > > @@ -6390,6 +6390,9 @@ if enabled vaapi; then
> > >> > > >   fi
> > >> > > >
> > >> > > >   check_cpp_condition vaapi_1 "va/va.h"
> > >> > > > "VA_CHECK_VERSION(1, 0,
> > >> 0)"
> > >> > > > +if ! check_struct "va/va.h" "struct _VAProcPipelineCaps"
> > >> > rotation_flags;
> > >> > > then
> > >> > > > +disable rotation_vaapi_filter
> > >> > > > +fi
> > >> > > >   fi
> > >> > > >
> > >> > > >   if enabled_all opencl libdrm ; then diff --git
> > >> > > > a/libavfilter/Makefile b/libavfilter/Makefile index
> > >> > > > 108a2f87d7..534650364a 100644
> > >> > > > --- a/libavfilter/Makefile
> > >> > > > +++ b/libavfilter/Makefile
> > >> > > > @@ -349,6 +349,7 @@ OBJS-$(CONFIG_SETRANGE_FILTER)
> +=
> > >> > > vf_setparams.o
> > >> > > >   OBJS-$(CONFIG_SETSAR_FILTER) += vf_aspect.o
> > >> > > >   OBJS-$(CONFIG_SETTB_FILTER)  += settb.o
> > >> > > >   OBJS-$(CONFIG_SHARPNESS_VAAPI_FILTER)+= vf_misc_vaapi.o
> > >> > > vaapi_vpp.o
> > >> > > > +OBJS-$(CONFIG_ROTATION_VAAPI_FILTER) +=
> vf_rotate_vaapi.o
> > >> > > vaapi_vpp.o
> > >> > > >   OBJS-$(CONFIG_SHOWINFO_FILTER)   += vf_showinfo.o
> > >> > > >   OBJS-$(CONFIG_SHOWPALETTE_FILTER)+= vf_showpalette.o
> > >> > > >   OBJS-$(CONFIG_SHUFFLEFRAMES_FILTER)  +=
> > >> > > > vf_shuffleframes.o
> > >> > > > diff --git a/libavfilter/allfilters.c
> > >> > > > b/libavfilter/allfilters.c index
> > >> > > > 557590850b..4b90a7f440 100644
> > >> > > > --- a/libavfilter/allfilters.c
> > >> > > > +++ b/libavfilter/allfilters.c
> > >> > > > @@ -333,6 +333,7 @@ extern AVFilter ff_vf_setrange;
> > >> > > >   extern AVFilter ff_vf_setsar;
> > >> > > >   extern AVFilter ff_vf_settb;
> > >> > > >   extern AVFilter ff_vf_sharpness_vaapi;
> > >> > > > +extern AVFilter ff_vf_rotation_vaapi;
> > >> > > >   extern AVFilter ff_vf_showinfo;
> > >> > > >   extern AVFilter ff_vf_showpalette;
> > >> > > >   extern AVFilter ff_vf_shuffleframes; diff --git
> > >> > > > a/libavfilter/vaapi_vpp.h b/libavfilter/vaapi_vpp.h index
> > >> > > > 0bc31018d4..cfe19b689f 100644
> > >> > > > --- a/libavfilter/vaapi_vpp.h
> > >> > > > +++ b/libavfilter/vaapi_vpp.h
> > >> > > > @@ -44,6 +44,7 @@ typedef struct VAAPIVPPContext {
> > >> > > >   int output_width;   // computed width
> > >> > > >   int output_height;  // computed height
> > >> > > >
> > >> > > > +in

[FFmpeg-devel] [PATCH v2 0/1] avcodec/mf: implemented Media Foundation wrapper

2018-10-29 Thread Paweł Wegner

Thanks for the tip; I took Media Foundation support from PLEX's FFmpeg fork.
The newer patch revision contains hw decoding through Media Foundation.

Paweł Wegner (1):
  avcodec/mf: implemented Media Foundation wrapper

 configure  |   48 +-
 fftools/Makefile   |1 +
 fftools/ffmpeg.h   |2 +
 fftools/ffmpeg_mf.c|  116 ++
 fftools/ffmpeg_opt.c   |3 +
 libavcodec/Makefile|1 +
 libavcodec/allcodecs.c |   28 +
 libavcodec/hwaccels.h  |   12 +
 libavcodec/mf.c| 2160 
 libavcodec/mf_utils.c  |  734 +++
 libavcodec/mf_utils.h  |  207 +++
 libavutil/Makefile |3 +
 libavutil/hwcontext.c  |3 +
 libavutil/hwcontext.h  |1 +
 libavutil/hwcontext_internal.h |1 +
 libavutil/hwcontext_mf.c   |  440 +++
 libavutil/hwcontext_mf.h   |  122 ++
 libavutil/pixdesc.c|4 +
 libavutil/pixfmt.h |2 +
 19 files changed, 3887 insertions(+), 1 deletion(-)
 create mode 100644 fftools/ffmpeg_mf.c
 create mode 100644 libavcodec/mf.c
 create mode 100644 libavcodec/mf_utils.c
 create mode 100644 libavcodec/mf_utils.h
 create mode 100644 libavutil/hwcontext_mf.c
 create mode 100644 libavutil/hwcontext_mf.h

-- 
2.17.1

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] libavformat/http.c: add the protection about the http seek implement if a http resource is not streamed, it can seek to the end of this resouce.I add the detail information at h

2018-10-29 Thread shenqichao

Signed-off-by: shenqichao 
---
 libavformat/http.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/http.c b/libavformat/http.c
index 3a35bc7eac..9401a5c63e 100644
--- a/libavformat/http.c
+++ b/libavformat/http.c
@@ -1669,7 +1669,7 @@ static int64_t http_seek_internal(URLContext *h, int64_t 
off, int whence, int fo
 int old_buf_size, ret;
 AVDictionary *options = NULL;
 
-if (whence == AVSEEK_SIZE)
+if (whence == AVSEEK_SIZE || (h->is_streamed == 0 &&  whence == SEEK_SET 
&& off == s->filesize))
 return s->filesize;
 else if (!force_reconnect &&
  ((whence == SEEK_CUR && off == 0) ||
-- 
2.19.0


___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] lavc/mjpegdec: fix VA-API MJPEG decoding uninitialized huffman table

2018-10-29 Thread Jun Zhao

From: Jun Zhao 

Now VA-API HWAccel MJPEG decoding uninitialized huffman table, it's
will lead to the decoding error like"Failed to sync surface 0x5: 23
(internal decoding error)." in iHD open source driver.

Signed-off-by: dlin2 
Signed-off-by: Jun Zhao 
---
 libavcodec/mjpegdec.c |   19 +++
 1 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c
index b0cb3ff..89effb6 100644
--- a/libavcodec/mjpegdec.c
+++ b/libavcodec/mjpegdec.c
@@ -75,6 +75,25 @@ static int build_vlc(VLC *vlc, const uint8_t *bits_table,
 static int build_basic_mjpeg_vlc(MJpegDecodeContext *s)
 {
 int ret;
+int i;
+
+/* initialize default huffman tables */
+for (i = 0; i < 16; i++)
+s->raw_huffman_lengths[0][0][i] = avpriv_mjpeg_bits_dc_luminance[i + 
1];
+for (i = 0; i < 12; i++)
+s->raw_huffman_values[0][0][i] = avpriv_mjpeg_val_dc[i];
+for (i = 0; i < 16; i++)
+s->raw_huffman_lengths[0][1][i] = avpriv_mjpeg_bits_dc_chrominance[i + 
1];
+for (i = 0; i < 12; i++)
+s->raw_huffman_values[0][1][i] = avpriv_mjpeg_val_dc[i];
+for (i = 0; i < 16; i++)
+s->raw_huffman_lengths[1][0][i] = avpriv_mjpeg_bits_ac_luminance[i + 
1];
+for (i = 0; i < 162; i++)
+s->raw_huffman_values[1][0][i] = avpriv_mjpeg_val_ac_luminance[i];
+for (i = 0; i < 16; i++)
+s->raw_huffman_lengths[1][1][i] = avpriv_mjpeg_bits_ac_chrominance[i + 
1];
+for (i = 0; i < 162; i++)
+s->raw_huffman_values[1][1][i] = avpriv_mjpeg_val_ac_chrominance[i];
 
 if ((ret = build_vlc(&s->vlcs[0][0], avpriv_mjpeg_bits_dc_luminance,
  avpriv_mjpeg_val_dc, 12, 0, 0)) < 0)
-- 
1.7.1

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavc/mjpegdec: fix VA-API MJPEG decoding uninitialized huffman table

2018-10-29 Thread Li, Zhong

> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of Jun Zhao
> Sent: Monday, October 29, 2018 6:26 PM
> To: ffmpeg-devel@ffmpeg.org
> Cc: Zhao, Jun ; Lin, Decai 
> Subject: [FFmpeg-devel] [PATCH] lavc/mjpegdec: fix VA-API MJPEG decoding
> uninitialized huffman table
> 
> From: Jun Zhao 
> 
> Now VA-API HWAccel MJPEG decoding uninitialized huffman table, it's will
> lead to the decoding error like"Failed to sync surface 0x5: 23 (internal
> decoding error)." in iHD open source driver.
> 
> Signed-off-by: dlin2 
> Signed-off-by: Jun Zhao 
> ---
>  libavcodec/mjpegdec.c |   19 +++
>  1 files changed, 19 insertions(+), 0 deletions(-)
> 
> diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c index
> b0cb3ff..89effb6 100644
> --- a/libavcodec/mjpegdec.c
> +++ b/libavcodec/mjpegdec.c
> @@ -75,6 +75,25 @@ static int build_vlc(VLC *vlc, const uint8_t *bits_table,
> static int build_basic_mjpeg_vlc(MJpegDecodeContext *s)  {
>  int ret;
> +int i;
> +
> +/* initialize default huffman tables */
> +for (i = 0; i < 16; i++)
> +s->raw_huffman_lengths[0][0][i] =
> avpriv_mjpeg_bits_dc_luminance[i + 1];
> +for (i = 0; i < 12; i++)
> +s->raw_huffman_values[0][0][i] = avpriv_mjpeg_val_dc[i];
> +for (i = 0; i < 16; i++)
> +s->raw_huffman_lengths[0][1][i] =
> avpriv_mjpeg_bits_dc_chrominance[i + 1];
> +for (i = 0; i < 12; i++)
> +s->raw_huffman_values[0][1][i] = avpriv_mjpeg_val_dc[i];
> +for (i = 0; i < 16; i++)
> +s->raw_huffman_lengths[1][0][i] =
> avpriv_mjpeg_bits_ac_luminance[i + 1];
> +for (i = 0; i < 162; i++)
> +s->raw_huffman_values[1][0][i] =
> avpriv_mjpeg_val_ac_luminance[i];
> +for (i = 0; i < 16; i++)
> +s->raw_huffman_lengths[1][1][i] =
> avpriv_mjpeg_bits_ac_chrominance[i + 1];
> +for (i = 0; i < 162; i++)
> +s->raw_huffman_values[1][1][i] =
> + avpriv_mjpeg_val_ac_chrominance[i];
> 
>  if ((ret = build_vlc(&s->vlcs[0][0], avpriv_mjpeg_bits_dc_luminance,
>   avpriv_mjpeg_val_dc, 12, 0, 0)) < 0)
> --
> 1.7.1

Should this be fixed in iHD driver instead of ffmpeg? 
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/2] lavc/amfenc: moving amf common code (library and context) to lavu/hwcontext_amf from amfenc to be reused in other amf components

2018-10-29 Thread Alexander Kravchenko

Hi, Mark.
Thanks for review.
Could you please check the following comments/questions?

> +
> > +static const AVClass amflib_class = {
> > +.class_name = "amf",
> > +.item_name = av_default_item_name,
> > +.version = LIBAVUTIL_VERSION_INT,
> > +};
>
> This class shouldn't be needed - the right class to use is the one in the
> AVHWDeviceContext, you should be able to pass it to the right place via
> your AMFDeviceContextPrivate structure.
>
> > +
> > +typedef struct AMFLibraryContext {
> > +const AVClass  *avclass;
> > +} AMFLibraryContext;
> > +
> > +static AMFLibraryContext amflib_context =
> > +{
> > +.avclass = &amflib_class,
> > +};
>
> This structure is just a dummy for the class?  Use the AVHWDeviceContext.
>
> > +
> > +typedef struct AmfTraceWriter {
> > +const AMFTraceWriterVtbl*vtbl;
> > +void*avcl;
> > +} AmfTraceWriter;
> > +
> > +static void AMF_CDECL_CALL AMFTraceWriter_Write(AMFTraceWriter *pThis,
>
> It would be sensible to take the opportunity to fix the function name to
> conform to ffmpeg style.
>
> > +const wchar_t *scope, const wchar_t *message)
> > +{
> > +AmfTraceWriter *tracer = (AmfTraceWriter*)pThis;
> > +av_log(tracer->avcl, AV_LOG_DEBUG, "%ls: %ls", scope, message);
> > +}
> > +
> > +static void AMF_CDECL_CALL AMFTraceWriter_Flush(AMFTraceWriter *pThis)
> > +{
> > +}
> > +
> > +static const AMFTraceWriterVtbl tracer_vtbl =
> > +{
> > +.Write = AMFTraceWriter_Write,
> > +.Flush = AMFTraceWriter_Flush,
>
> Is this function really required to exist, given that it doesn't do
> anything?
>
> > +};
> > +
> > +static const AmfTraceWriter amf_trace_writer =
> > +{
> > +.vtbl = &tracer_vtbl,
> > +.avcl = &amflib_context,
> > +};
>
> This should probably be inside the AMFDeviceContextPrivate, so that it can
> point to the right context structure.
>
> This is the question.
AMF Library has global Trace settings, not per AMFContext object.
My intension was to create global AMF lib class and Tracer object which
refers to it as class parameter in av_log call.
It is required in scenario when multiple hwcontext_amf are created during
application lifecycle.
if this way is ok, should I add comments to code describes this? or is
there another way to have global object to handle this?

AMFTraceWriterVtbl.Flush - I am not sure that it can be set as null
pointer, I could double check with AMD developers.




> >  #include 
> >  #include 
>
> Kindof unrelated, but is there any reason why both of these are in the
> header rather than in the per-codec files?
>
>
Component management code is the same for all encoder components.
The only encoder id defines is used for component creation here.



>
> The log_to_dbg option is orphaned by this change.  Is it worth keeping?
> (If you want to keep it then maybe it could be a named option to
> av_hwdevice_ctx_create() -> amf_device_create().)
>
> log_to_dbg is removed from here, because this setting is global for AMF
library, not per component(encoder) or per AMFContext(hwcontext_amf object)

Probably I could implement some global AMF lib options which configures
tracer more precisely in general

Thanks,
Alexander
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH v2] libavdevice/libndi_newtek: Added extra_ips option to libndi_newtek allowing use remote network sources

2018-10-29 Thread Anton Platov

Signed-off-by: Anton Platov 
---
 doc/indevs.texi | 17 +
 libavdevice/libndi_newtek_dec.c |  4 +++-
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/doc/indevs.texi b/doc/indevs.texi
index 9a9cb69..c75c4b1 100644
--- a/doc/indevs.texi
+++ b/doc/indevs.texi
@@ -1078,6 +1078,10 @@ Defaults to @option{0.5}.
 When this flag is @option{false}, all video that you receive will be 
progressive.
 Defaults to @option{true}.
 
+@item extra_ips
+If is set to list of comma separated ip addresses, scan for sources not only 
+using mDNS but also use unicast ip addresses specified by this list.
+
 @end table
 
 @subsection Examples
@@ -1091,11 +1095,24 @@ ffmpeg -f libndi_newtek -find_sources 1 -i dummy
 @end example
 
 @item
+List local and remote input devices:
+@example
+ffmpeg -f libndi_newtek -extra_ips "192.168.10.10" -find_sources 1 -i dummy
+@end example
+
+@item
 Restream to NDI:
 @example
 ffmpeg -f libndi_newtek -i "DEV-5.INTERNAL.M1STEREO.TV (NDI_SOURCE_NAME_1)" -f 
libndi_newtek -y NDI_SOURCE_NAME_2
 @end example
 
+@item
+Restream remote NDI to local NDI:
+@example
+ffmpeg -f libndi_newtek -extra_ips "192.168.10.10" -i 
"DEV-5.REMOTE.M1STEREO.TV (NDI_SOURCE_NAME_1)" -f libndi_newtek -y 
NDI_SOURCE_NAME_2
+@end example
+
+
 @end itemize
 
 @section openal
diff --git a/libavdevice/libndi_newtek_dec.c b/libavdevice/libndi_newtek_dec.c
index 4fb7197..d2d5648 100644
--- a/libavdevice/libndi_newtek_dec.c
+++ b/libavdevice/libndi_newtek_dec.c
@@ -33,6 +33,7 @@ struct NDIContext {
 int find_sources;
 int64_t wait_sources;
 int allow_video_fields;
+char *extra_ips;
 
 /* Runtime */
 NDIlib_recv_create_t *recv;
@@ -99,7 +100,7 @@ static int ndi_find_sources(AVFormatContext *avctx, const 
char *name, NDIlib_sou
 struct NDIContext *ctx = avctx->priv_data;
 const NDIlib_source_t *ndi_srcs = NULL;
 const NDIlib_find_create_t find_create_desc = { .show_local_sources = true,
-.p_groups = NULL, .p_extra_ips = NULL };
+.p_groups = NULL, .p_extra_ips = ctx->extra_ips };
 
 if (!ctx->ndi_find)
 ctx->ndi_find = NDIlib_find_create2(&find_create_desc);
@@ -317,6 +318,7 @@ static const AVOption options[] = {
 { "find_sources", "Find available sources"  , OFFSET(find_sources), 
AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, DEC },
 { "wait_sources", "Time to wait until the number of online sources have 
changed"  , OFFSET(wait_sources), AV_OPT_TYPE_DURATION, { .i64 = 100 }, 
10, 2000, DEC },
 { "allow_video_fields", "When this flag is FALSE, all video that you 
receive will be progressive"  , OFFSET(allow_video_fields), AV_OPT_TYPE_BOOL, { 
.i64 = 1 }, 0, 1, DEC },
+{ "extra_ips", "List of comma separated ip addresses to scan for remote 
sources",   OFFSET(extra_ips), AV_OPT_TYPE_STRING, {.str = NULL }, 0, 0, 
DEC },
 { NULL },
 };
 
-- 
2.7.4

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] libavformat: add multiple PCR streams in MPEGTS muxer

2018-10-29 Thread Andrey Turkin

This looks like a (strange) hack to me.
First, you cannot just throw some pids out - that will make a non-standard
compliant stream. PMT will signal missing streams; PCR pid will be missing
also. So let's assume that your tool can also rewrite PMT.
Secondly, there is supposed to be one and only one PCR stream per program;
there is no way to signal multiple PCRs. So let's assume you have some
out-of-band list of PCRs for this particular case (or maybe you can just
assume there are PCR marks in each stream so you can configure your remuxer
accordingly).
Thirdly and most importantly, I might be wrong here but it looks like your
patch just "spreads" same PCRs which would go into single stream over all
program's streams, i.e. overall PCR count doesn't change. Which means that
remuxed stream might have arbitrarily large inter-PCR gaps which is pretty
bad (assuming you care about PCRs at all, which presumably you do).

Also - do you really have multiple audio streams in single program, which
are supposed to be remuxed as separate radio channels? Or same for multiple
video streams? Is this for ABR purposes? Can you describe your use case in
some more details?

пн, 29 окт. 2018 г. в 4:20, :

> From 44c9f3d96c5969349bd2dc4d63e9a8b8e6b5a0e2 Mon Sep 17 00:00:00 2001
> From: M. Sanders 
> Date: Sat, 27 Oct 2018 21:39:44 +0200
> Subject: [PATCH] libavformat: add multiple PCR streams in MPEGTS muxer
>
> ---
> This simple patch adds two new options to the MPEG-TS muxer.
> You can use them to mark PCRs in additional VIDEO and/or AUDIO streams.
> It's useful when you will remux the output with pid filtering tools.
> For example, if you put PCR in all audio streams, then you can filter
> each audio stream to create a radio service.
> PCR marks do not disturb outgoing stream, but it is not recommended
> to do so as a regular rule.
>
>  doc/muxers.texi |  6 ++
>  libavformat/mpegtsenc.c | 19 +--
>  2 files changed, 23 insertions(+), 2 deletions(-)
>
> diff --git a/doc/muxers.texi b/doc/muxers.texi
> index f18543e83d..328707d0ed 100644
> --- a/doc/muxers.texi
> +++ b/doc/muxers.texi
> @@ -1528,6 +1528,12 @@ is @code{-1}, which results in shifting timestamps
> so that they start from 0.
>  @item omit_video_pes_length @var{boolean}
>  Omit the PES packet length for video packets. Default is @code{1} (true).
>
> +@item pcr_all_video @var{boolean}
> +Include PCR values in all video streams. Default is @code{0} (false).
> +
> +@item pcr_all_audio @var{boolean}
> +Include PCR values in all audio streams. Default is @code{0} (false).
> +
>  @item pcr_period @var{integer}
>  Override the default PCR retransmission time in milliseconds. Ignored if
>  variable muxrate is selected. Default is @code{20}.
> diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c
> index 3339e26d50..caef817a3d 100644
> --- a/libavformat/mpegtsenc.c
> +++ b/libavformat/mpegtsenc.c
> @@ -113,6 +113,8 @@ typedef struct MpegTSWrite {
>  double sdt_period;
>  int64_t last_pat_ts;
>  int64_t last_sdt_ts;
> +int pcr_all_video;
> +int pcr_all_audio;
>
>  int omit_video_pes_length;
>  } MpegTSWrite;
> @@ -1178,11 +1180,18 @@ static void mpegts_write_pes(AVFormatContext *s,
> AVStream *st,
>  int64_t pcr = -1; /* avoid warning */
>  int64_t delay = av_rescale(s->max_delay, 9, AV_TIME_BASE);
>  int force_pat = st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO && key
> && !ts_st->prev_payload_key;
> +int force_pcr = 0;
>
>  av_assert0(ts_st->payload != buf || st->codecpar->codec_type !=
> AVMEDIA_TYPE_VIDEO);
>  if (ts->flags & MPEGTS_FLAG_PAT_PMT_AT_FRAMES &&
> st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO) {
>  force_pat = 1;
>  }
> +if (ts->pcr_all_video && st->codecpar->codec_type ==
> AVMEDIA_TYPE_VIDEO) {
> +force_pcr = 1;
> +}
> +if (ts->pcr_all_audio && st->codecpar->codec_type ==
> AVMEDIA_TYPE_AUDIO) {
> +force_pcr = 1;
> +}
>
>  is_start = 1;
>  while (payload_size > 0) {
> @@ -1190,7 +1199,7 @@ static void mpegts_write_pes(AVFormatContext *s,
> AVStream *st,
>  force_pat = 0;
>
>  write_pcr = 0;
> -if (ts_st->pid == ts_st->service->pcr_pid) {
> +if (ts_st->pid == ts_st->service->pcr_pid || force_pcr) {
>  if (ts->mux_rate > 1 || is_start) // VBR pcr period is based
> on frames
>  ts_st->service->pcr_packet_count++;
>  if (ts_st->service->pcr_packet_count >=
> @@ -1228,7 +1237,7 @@ static void mpegts_write_pes(AVFormatContext *s,
> AVStream *st,
>  }
>  if (key && is_start && pts != AV_NOPTS_VALUE) {
>  // set Random Access for key frames
> -if (ts_st->pid == ts_st->service->pcr_pid)
> +if (ts_st->pid == ts_st->service->pcr_pid || force_pcr)
>  write_pcr = 1;
>  set_af_flag(buf, 0x40);
>  q = get_ts_payload_start(buf);
> @@ -1951,6 +1960,12 @@ static const AVOptio

Re: [FFmpeg-devel] [PATCH 1/2] libavutil: Undeprecate the AVFrame reordered_opaque field

2018-10-29 Thread Derek Buitenhuis

On 25/10/2018 13:58, Martin Storsjö wrote:
> This was marked as deprecated (but only in the doxygen, not with an
> actual deprecation attribute) in 81c623fae05 in 2011, but was
> undeprecated in ad1ee5fa7.
> ---
>   libavutil/frame.h   | 1 -
>   libavutil/version.h | 2 +-
>   2 files changed, 1 insertion(+), 2 deletions(-)

I don't understand why this is being used in favour of a proper
pointer field? An integer field is just ascting to be misused.
Even the doxygen is really sketchy on it.

I also don't understand why this is at the AVCodecContext level
and not packet/frame?

- Derek
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] fate/api-h264-slice-test: use cleaner error handling

2018-10-29 Thread joshdk

From: Josh de Kock 

---
 tests/api/api-h264-slice-test.c | 74 +++--
 1 file changed, 43 insertions(+), 31 deletions(-)

diff --git a/tests/api/api-h264-slice-test.c b/tests/api/api-h264-slice-test.c
index 57e7dc79c3..08d5d57941 100644
--- a/tests/api/api-h264-slice-test.c
+++ b/tests/api/api-h264-slice-test.c
@@ -48,7 +48,7 @@
 
 static int header = 0;
 
-static void decode(AVCodecContext *dec_ctx, AVFrame *frame,
+static int decode(AVCodecContext *dec_ctx, AVFrame *frame,
AVPacket *pkt)
 {
 static uint64_t frame_cnt = 0;
@@ -57,20 +57,20 @@ static void decode(AVCodecContext *dec_ctx, AVFrame *frame,
 ret = avcodec_send_packet(dec_ctx, pkt);
 if (ret < 0) {
 fprintf(stderr, "Error sending a packet for decoding: %s\n", 
av_err2str(ret));
-exit(1);
+return ret;
 }
 
 while (ret >= 0) {
 const AVPixFmtDescriptor *desc;
-char *sum;
+char sum[AV_HASH_MAX_SIZE * 2 + 1];
 struct AVHashContext *hash;
 
 ret = avcodec_receive_frame(dec_ctx, frame);
 if (ret == AVERROR(EAGAIN) || ret == AVERROR_EOF) {
-return;
+return 0;
 } else if (ret < 0) {
 fprintf(stderr, "Error during decoding: %s\n", av_err2str(ret));
-exit(1);
+return ret;
 }
 
 if (!header) {
@@ -87,9 +87,10 @@ static void decode(AVCodecContext *dec_ctx, AVFrame *frame,
 header = 1;
 }
 desc = av_pix_fmt_desc_get(dec_ctx->pix_fmt);
-av_hash_alloc(&hash, "md5");
+if ((ret = av_hash_alloc(&hash, "md5")) < 0) {
+return ret;
+}
 av_hash_init(hash);
-sum = av_mallocz(av_hash_get_size(hash) * 2 + 1);
 
 for (int i = 0; i < frame->height; i++)
 av_hash_update(hash, &frame->data[0][i * frame->linesize[0]], 
frame->width);
@@ -104,8 +105,8 @@ static void decode(AVCodecContext *dec_ctx, AVFrame *frame,
 (frame->width * frame->height + 2 * (frame->height >> 
desc->log2_chroma_h) * (frame->width >> desc->log2_chroma_w)), sum);
 frame_cnt += 1;
 av_hash_freep(&hash);
-av_free(sum);
 }
+return 0;
 }
 
 int main(int argc, char **argv)
@@ -117,12 +118,12 @@ int main(int argc, char **argv)
 AVPacket *pkt;
 FILE *fd;
 char nal[MAX_SLICES * UINT16_MAX + AV_INPUT_BUFFER_PADDING_SIZE];
-int nals = 0;
+int nals = 0, ret = 0;
 char *p = nal;
 
 if (argc < 4) {
 fprintf(stderr, "Usage: %s   \n", 
argv[0]);
-exit(1);
+return -1;
 }
 
 if (!(threads = strtoul(argv[1], NULL, 0)))
@@ -134,17 +135,19 @@ int main(int argc, char **argv)
 setmode(fileno(stdout), O_BINARY);
 #endif
 
-if (!(pkt = av_packet_alloc()))
-exit(1);
+if (!(pkt = av_packet_alloc())) {
+return -1;
+}
 
 if (!(codec = avcodec_find_decoder(AV_CODEC_ID_H264))) {
 fprintf(stderr, "Codec not found\n");
-exit(1);
+return -1;
 }
 
 if (!(c = avcodec_alloc_context3(codec))) {
 fprintf(stderr, "Could not allocate video codec context\n");
-exit(1);
+ret = -1;
+goto err_avctx;
 }
 
 c->width  = 352;
@@ -154,15 +157,16 @@ int main(int argc, char **argv)
 c->thread_type = FF_THREAD_SLICE;
 c->thread_count = threads;
 
-if (avcodec_open2(c, codec, NULL) < 0) {
+if ((ret = avcodec_open2(c, codec, NULL)) < 0) {
 fprintf(stderr, "Could not open codec\n");
-exit(1);
+goto err_frame;
 }
 
 #if HAVE_THREADS
 if (c->active_thread_type != FF_THREAD_SLICE) {
 fprintf(stderr, "Couldn't activate slice threading: %d\n", 
c->active_thread_type);
-exit(1);
+ret = -1;
+goto err_frame;
 }
 #else
 fprintf(stderr, "WARN: not using threads, only checking decoding slice 
NALUs\n");
@@ -170,34 +174,36 @@ int main(int argc, char **argv)
 
 if (!(frame = av_frame_alloc())) {
 fprintf(stderr, "Could not allocate video frame\n");
-exit(1);
+ret = -1;
+goto err_frame;
 }
 
 if (!(fd = fopen(argv[2], "rb"))) {
 fprintf(stderr, "Couldn't open NALU file: %s\n", argv[2]);
-exit(1);
+ret = -1;
+goto err_fopen;
 }
 
 while(1) {
 uint16_t size = 0;
-ssize_t ret = fread(&size, 1, sizeof(uint16_t), fd);
-if (ret < 0) {
-perror("Couldn't read size");
-exit(1);
-} else if (ret != sizeof(uint16_t))
+size_t ret = fread(&size, 1, sizeof(uint16_t), fd);
+if (ret != sizeof(uint16_t))
 break;
 size = ntohs(size);
 ret = fread(p, 1, size, fd);
-if (ret < 0 || ret != size) {
+if (ret != size) {
 perror("Couldn't read data");
-exit(1);
+goto err;
 }
 p += ret;
 
 if (++nals >= threads) {
+int decret =

Re: [FFmpeg-devel] [PATCH 1/2] libavutil: Undeprecate the AVFrame reordered_opaque field

2018-10-29 Thread Martin Storsjö


On Mon, 29 Oct 2018, Derek Buitenhuis wrote:


On 25/10/2018 13:58, Martin Storsjö wrote:

This was marked as deprecated (but only in the doxygen, not with an
actual deprecation attribute) in 81c623fae05 in 2011, but was
undeprecated in ad1ee5fa7.
---
  libavutil/frame.h   | 1 -
  libavutil/version.h | 2 +-
  2 files changed, 1 insertion(+), 2 deletions(-)


I don't understand why this is being used in favour of a proper
pointer field? An integer field is just ascting to be misused.
Even the doxygen is really sketchy on it.


It's essentially meant to be used as union { ptr; int64_t } assuming you 
don't have pointers larger than 64 bits.



I also don't understand why this is at the AVCodecContext level
and not packet/frame?


It is on the frame level, but not in the packet struct (probably for 
historical reasons) - instead of in the packet, it's in AVCodecContext. 
For decoding, you set the value in AVCodecContext before feeding packets 
to it, and get the corresponding value reordered into the output AVFrame. 
If things were to be redone from scratch, moving it into AVPacket would 
probably make more sense, but there's not much point in doing that right 
now.


At some point, the doxygen got markers saying this mechanism was 
deprecated and one should use the new pkt_pts instead. Before that, 
reordered_opaque was mainly used for getting reordered pts as there 
was no other mechanism for it.


But even with the proper pkt_pts field, having a generic opaque field that 
travels along with the reordering is useful, which is why the deprecation 
doxygen comments were removed in ad1ee5fa7. But that commit just missed to 
remove one of the doxygen deprecation.


// Martin
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] fate/api-h264-slice-test: use cleaner error handling

2018-10-29 Thread James Almer

On 10/29/2018 10:57 AM, jos...@ob-encoder.com wrote:
> From: Josh de Kock 
> 
> ---
>  tests/api/api-h264-slice-test.c | 74 +++--
>  1 file changed, 43 insertions(+), 31 deletions(-)
> 
> diff --git a/tests/api/api-h264-slice-test.c b/tests/api/api-h264-slice-test.c
> index 57e7dc79c3..08d5d57941 100644
> --- a/tests/api/api-h264-slice-test.c
> +++ b/tests/api/api-h264-slice-test.c
> @@ -48,7 +48,7 @@
>  
>  static int header = 0;
>  
> -static void decode(AVCodecContext *dec_ctx, AVFrame *frame,
> +static int decode(AVCodecContext *dec_ctx, AVFrame *frame,
> AVPacket *pkt)
>  {
>  static uint64_t frame_cnt = 0;
> @@ -57,20 +57,20 @@ static void decode(AVCodecContext *dec_ctx, AVFrame 
> *frame,
>  ret = avcodec_send_packet(dec_ctx, pkt);
>  if (ret < 0) {
>  fprintf(stderr, "Error sending a packet for decoding: %s\n", 
> av_err2str(ret));
> -exit(1);
> +return ret;
>  }
>  
>  while (ret >= 0) {
>  const AVPixFmtDescriptor *desc;
> -char *sum;
> +char sum[AV_HASH_MAX_SIZE * 2 + 1];
>  struct AVHashContext *hash;
>  
>  ret = avcodec_receive_frame(dec_ctx, frame);
>  if (ret == AVERROR(EAGAIN) || ret == AVERROR_EOF) {
> -return;
> +return 0;
>  } else if (ret < 0) {
>  fprintf(stderr, "Error during decoding: %s\n", av_err2str(ret));
> -exit(1);
> +return ret;
>  }
>  
>  if (!header) {
> @@ -87,9 +87,10 @@ static void decode(AVCodecContext *dec_ctx, AVFrame *frame,
>  header = 1;
>  }
>  desc = av_pix_fmt_desc_get(dec_ctx->pix_fmt);
> -av_hash_alloc(&hash, "md5");
> +if ((ret = av_hash_alloc(&hash, "md5")) < 0) {
> +return ret;
> +}
>  av_hash_init(hash);
> -sum = av_mallocz(av_hash_get_size(hash) * 2 + 1);
>  
>  for (int i = 0; i < frame->height; i++)
>  av_hash_update(hash, &frame->data[0][i * frame->linesize[0]], 
> frame->width);
> @@ -104,8 +105,8 @@ static void decode(AVCodecContext *dec_ctx, AVFrame 
> *frame,
>  (frame->width * frame->height + 2 * (frame->height >> 
> desc->log2_chroma_h) * (frame->width >> desc->log2_chroma_w)), sum);
>  frame_cnt += 1;
>  av_hash_freep(&hash);
> -av_free(sum);
>  }
> +return 0;
>  }
>  
>  int main(int argc, char **argv)
> @@ -117,12 +118,12 @@ int main(int argc, char **argv)
>  AVPacket *pkt;
>  FILE *fd;
>  char nal[MAX_SLICES * UINT16_MAX + AV_INPUT_BUFFER_PADDING_SIZE];
> -int nals = 0;
> +int nals = 0, ret = 0;
>  char *p = nal;
>  
>  if (argc < 4) {
>  fprintf(stderr, "Usage: %s   \n", 
> argv[0]);
> -exit(1);
> +return -1;
>  }
>  
>  if (!(threads = strtoul(argv[1], NULL, 0)))
> @@ -134,17 +135,19 @@ int main(int argc, char **argv)
>  setmode(fileno(stdout), O_BINARY);
>  #endif
>  
> -if (!(pkt = av_packet_alloc()))
> -exit(1);
> +if (!(pkt = av_packet_alloc())) {
> +return -1;
> +}
>  
>  if (!(codec = avcodec_find_decoder(AV_CODEC_ID_H264))) {
>  fprintf(stderr, "Codec not found\n");
> -exit(1);
> +return -1;

You're leaking the AVPacket if you return here.

>  }
>  
>  if (!(c = avcodec_alloc_context3(codec))) {
>  fprintf(stderr, "Could not allocate video codec context\n");
> -exit(1);
> +ret = -1;
> +goto err_avctx;
>  }
>  
>  c->width  = 352;
> @@ -154,15 +157,16 @@ int main(int argc, char **argv)
>  c->thread_type = FF_THREAD_SLICE;
>  c->thread_count = threads;
>  
> -if (avcodec_open2(c, codec, NULL) < 0) {
> +if ((ret = avcodec_open2(c, codec, NULL)) < 0) {
>  fprintf(stderr, "Could not open codec\n");
> -exit(1);
> +goto err_frame;
>  }
>  
>  #if HAVE_THREADS
>  if (c->active_thread_type != FF_THREAD_SLICE) {
>  fprintf(stderr, "Couldn't activate slice threading: %d\n", 
> c->active_thread_type);
> -exit(1);
> +ret = -1;
> +goto err_frame;
>  }
>  #else
>  fprintf(stderr, "WARN: not using threads, only checking decoding slice 
> NALUs\n");
> @@ -170,34 +174,36 @@ int main(int argc, char **argv)
>  
>  if (!(frame = av_frame_alloc())) {
>  fprintf(stderr, "Could not allocate video frame\n");
> -exit(1);
> +ret = -1;
> +goto err_frame;
>  }
>  
>  if (!(fd = fopen(argv[2], "rb"))) {
>  fprintf(stderr, "Couldn't open NALU file: %s\n", argv[2]);
> -exit(1);
> +ret = -1;
> +goto err_fopen;
>  }
>  
>  while(1) {
>  uint16_t size = 0;
> -ssize_t ret = fread(&size, 1, sizeof(uint16_t), fd);
> -if (ret < 0) {
> -perror("Couldn't read size");
> -exit(1);
> -} else if (ret != sizeof(uint16_t))
> +siz

[FFmpeg-devel] [PATCH v2] fate/api-h264-slice-test: use cleaner error handling

2018-10-29 Thread joshdk

From: Josh de Kock 

---
 tests/api/api-h264-slice-test.c | 85 +++--
 1 file changed, 48 insertions(+), 37 deletions(-)

diff --git a/tests/api/api-h264-slice-test.c b/tests/api/api-h264-slice-test.c
index 57e7dc79c3..b3f9f91ab3 100644
--- a/tests/api/api-h264-slice-test.c
+++ b/tests/api/api-h264-slice-test.c
@@ -48,7 +48,7 @@
 
 static int header = 0;
 
-static void decode(AVCodecContext *dec_ctx, AVFrame *frame,
+static int decode(AVCodecContext *dec_ctx, AVFrame *frame,
AVPacket *pkt)
 {
 static uint64_t frame_cnt = 0;
@@ -57,20 +57,20 @@ static void decode(AVCodecContext *dec_ctx, AVFrame *frame,
 ret = avcodec_send_packet(dec_ctx, pkt);
 if (ret < 0) {
 fprintf(stderr, "Error sending a packet for decoding: %s\n", 
av_err2str(ret));
-exit(1);
+return ret;
 }
 
 while (ret >= 0) {
 const AVPixFmtDescriptor *desc;
-char *sum;
+char sum[AV_HASH_MAX_SIZE * 2 + 1];
 struct AVHashContext *hash;
 
 ret = avcodec_receive_frame(dec_ctx, frame);
 if (ret == AVERROR(EAGAIN) || ret == AVERROR_EOF) {
-return;
+return 0;
 } else if (ret < 0) {
 fprintf(stderr, "Error during decoding: %s\n", av_err2str(ret));
-exit(1);
+return ret;
 }
 
 if (!header) {
@@ -87,9 +87,10 @@ static void decode(AVCodecContext *dec_ctx, AVFrame *frame,
 header = 1;
 }
 desc = av_pix_fmt_desc_get(dec_ctx->pix_fmt);
-av_hash_alloc(&hash, "md5");
+if ((ret = av_hash_alloc(&hash, "md5")) < 0) {
+return ret;
+}
 av_hash_init(hash);
-sum = av_mallocz(av_hash_get_size(hash) * 2 + 1);
 
 for (int i = 0; i < frame->height; i++)
 av_hash_update(hash, &frame->data[0][i * frame->linesize[0]], 
frame->width);
@@ -104,25 +105,25 @@ static void decode(AVCodecContext *dec_ctx, AVFrame 
*frame,
 (frame->width * frame->height + 2 * (frame->height >> 
desc->log2_chroma_h) * (frame->width >> desc->log2_chroma_w)), sum);
 frame_cnt += 1;
 av_hash_freep(&hash);
-av_free(sum);
 }
+return 0;
 }
 
 int main(int argc, char **argv)
 {
-const AVCodec *codec;
+const AVCodec *codec = NULL;
 AVCodecContext *c = NULL;
-AVFrame *frame;
+AVFrame *frame = NULL;
 unsigned int threads;
 AVPacket *pkt;
-FILE *fd;
+FILE *file = NULL;
 char nal[MAX_SLICES * UINT16_MAX + AV_INPUT_BUFFER_PADDING_SIZE];
-int nals = 0;
+int nals = 0, ret = 0;
 char *p = nal;
 
 if (argc < 4) {
 fprintf(stderr, "Usage: %s   \n", 
argv[0]);
-exit(1);
+return -1;
 }
 
 if (!(threads = strtoul(argv[1], NULL, 0)))
@@ -134,17 +135,20 @@ int main(int argc, char **argv)
 setmode(fileno(stdout), O_BINARY);
 #endif
 
-if (!(pkt = av_packet_alloc()))
-exit(1);
+if (!(pkt = av_packet_alloc())) {
+return -1;
+}
 
 if (!(codec = avcodec_find_decoder(AV_CODEC_ID_H264))) {
 fprintf(stderr, "Codec not found\n");
-exit(1);
+ret = -1;
+goto err;
 }
 
 if (!(c = avcodec_alloc_context3(codec))) {
 fprintf(stderr, "Could not allocate video codec context\n");
-exit(1);
+ret = -1;
+goto err;
 }
 
 c->width  = 352;
@@ -154,15 +158,16 @@ int main(int argc, char **argv)
 c->thread_type = FF_THREAD_SLICE;
 c->thread_count = threads;
 
-if (avcodec_open2(c, codec, NULL) < 0) {
+if ((ret = avcodec_open2(c, codec, NULL)) < 0) {
 fprintf(stderr, "Could not open codec\n");
-exit(1);
+goto err;
 }
 
 #if HAVE_THREADS
 if (c->active_thread_type != FF_THREAD_SLICE) {
 fprintf(stderr, "Couldn't activate slice threading: %d\n", 
c->active_thread_type);
-exit(1);
+ret = -1;
+goto err;
 }
 #else
 fprintf(stderr, "WARN: not using threads, only checking decoding slice 
NALUs\n");
@@ -170,34 +175,36 @@ int main(int argc, char **argv)
 
 if (!(frame = av_frame_alloc())) {
 fprintf(stderr, "Could not allocate video frame\n");
-exit(1);
+ret = -1;
+goto err;
 }
 
-if (!(fd = fopen(argv[2], "rb"))) {
+if (!(file = fopen(argv[2], "rb"))) {
 fprintf(stderr, "Couldn't open NALU file: %s\n", argv[2]);
-exit(1);
+ret = -1;
+goto err;
 }
 
 while(1) {
 uint16_t size = 0;
-ssize_t ret = fread(&size, 1, sizeof(uint16_t), fd);
-if (ret < 0) {
-perror("Couldn't read size");
-exit(1);
-} else if (ret != sizeof(uint16_t))
+size_t ret = fread(&size, 1, sizeof(uint16_t), file);
+if (ret != sizeof(uint16_t))
 break;
 size = ntohs(size);
-ret = fread(p, 1, size, fd);
-if (ret < 0 || ret != size) {
+ret = fread(p, 1, size

Re: [FFmpeg-devel] [PATCH 1/2] libavutil: Undeprecate the AVFrame reordered_opaque field

2018-10-29 Thread Derek Buitenhuis

On 29/10/2018 14:10, Martin Storsjö wrote:
>> I don't understand why this is being used in favour of a proper
>> pointer field? An integer field is just ascting to be misused.
>> Even the doxygen is really sketchy on it.
> 
> It's essentially meant to be used as union { ptr; int64_t } assuming you
> don't have pointers larger than 64 bits.

It's not a union in the API, and I'm pretty sure that it violates the C spec
to use a unions to get an integer out of a pointer, shove it into an int64_t,
and then get it back out, and chnage it back via union. Especially for
32-bit pointers. It encourages terrible code.

I just don't think we should revive this as-is purely for convenience.

>> I also don't understand why this is at the AVCodecContext level
>> and not packet/frame?
> 
> It is on the frame level, but not in the packet struct (probably for
> historical reasons) - instead of in the packet, it's in AVCodecContext.
> For decoding, you set the value in AVCodecContext before feeding packets
> to it, and get the corresponding value reordered into the output AVFrame.
> If things were to be redone from scratch, moving it into AVPacket would
> probably make more sense, but there's not much point in doing that right
> now.

I mean, this is pretty gross, and non-obvious as far as I'm concerned.
Modifying the AVCodecContext on every call is just... eugh.

> At some point, the doxygen got markers saying this mechanism was
> deprecated and one should use the new pkt_pts instead. Before that,
> reordered_opaque was mainly used for getting reordered pts as there
> was no other mechanism for it.
> 
> But even with the proper pkt_pts field, having a generic opaque field that
> travels along with the reordering is useful, which is why the deprecation
> doxygen comments were removed in ad1ee5fa7. But that commit just missed to
> remove one of the doxygen deprecation.

I agree it's very useful, and something we should have, but not that we should
revive/use this partiular field... it's nasty.

- Derek
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 2/2] flvdec: Export unknown metadata packets as opaque data

2018-10-29 Thread Michael Niedermayer

On Sun, Oct 28, 2018 at 11:09:58PM +0200, Martin Storsjö wrote:
> ---
> Removed the option and made this behaviour the default.
> ---
>  libavformat/flv.h|  1 +
>  libavformat/flvdec.c | 18 ++
>  2 files changed, 15 insertions(+), 4 deletions(-)

should be ok

thanks

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

During times of universal deceit, telling the truth becomes a
revolutionary act. -- George Orwell


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 2/2] lavu/frame: Add error report if av_image_fill_pointers fail.

2018-10-29 Thread Michael Niedermayer

On Sun, Oct 28, 2018 at 11:05:47AM +0800, Jun Zhao wrote:
> Add error handle if av_image_fill_pointers fail.
> 
> Signed-off-by: Jun Zhao 
> ---
>  libavutil/frame.c |   10 ++
>  1 files changed, 6 insertions(+), 4 deletions(-)

should be ok

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I do not agree with what you have to say, but I'll defend to the death your
right to say it. -- Voltaire


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/3] avcodec/vp3: Do not initialize unused tables for keyframes in unpack_superblock()

2018-10-29 Thread Michael Niedermayer

On Sun, Oct 28, 2018 at 11:15:08AM -0300, James Almer wrote:
> On 10/28/2018 11:04 AM, Michael Niedermayer wrote:
> > Fixes: Timeout (139sec -> 102sec)
> > Fixes: 
> > 9642/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_VP3_fuzzer-6676767875006464
> > 
> > Found-by: continuous fuzzing process 
> > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer 
> > ---
> >  libavcodec/vp3.c | 16 +++-
> >  1 file changed, 15 insertions(+), 1 deletion(-)
> > 
> > diff --git a/libavcodec/vp3.c b/libavcodec/vp3.c
> > index 0e6da89abb..348416b25d 100644
> > --- a/libavcodec/vp3.c
> > +++ b/libavcodec/vp3.c
> > @@ -544,8 +544,21 @@ static int unpack_superblocks(Vp3DecodeContext *s, 
> > GetBitContext *gb)
> >   : s->y_superblock_count);
> >  int num_coded_frags = 0;
> >  
> > +if (s->keyframe) {
> > +for (i = sb_start; i < sb_end; i++) {
> > +/* iterate through all 16 fragments in a superblock */
> > +for (j = 0; j < 16; j++) {
> > +/* if the fragment is in bounds, check its coding 
> > status */
> > +current_fragment = s->superblock_fragments[i * 16 + j];
> > +if (current_fragment != -1) {
> > +s->coded_fragment_list[plane][num_coded_frags++] =
> > +current_fragment;
> > +}
> > +}
> > +}
> > +} else {
> >  for (i = sb_start; i < sb_end && get_bits_left(gb) > 0; i++) {
> > -if (s->keyframe == 0 && get_bits_left(gb) < 
> > plane0_num_coded_frags >> 2) {
> > +if (get_bits_left(gb) < plane0_num_coded_frags >> 2) {
> >  return AVERROR_INVALIDDATA;
> >  }
> >  /* iterate through all 16 fragments in a superblock */
> > @@ -580,6 +593,7 @@ static int unpack_superblocks(Vp3DecodeContext *s, 
> > GetBitContext *gb)
> >  }
> >  }
> >  }
> > +}
> >  if (!plane)
> >  plane0_num_coded_frags = num_coded_frags;
> >  s->total_num_coded_frags += num_coded_frags;
> 
> Please reindent before you push, either as a separate commit or directly
> in this one.

will add a commit that reindents
will apply

thanks
[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Old school: Use the lowest level language in which you can solve the problem
conveniently.
New school: Use the highest level language in which the latest supercomputer
can solve the problem without the user falling asleep waiting.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] avfilter: add (a)graphmonitor filter(s)

2018-10-29 Thread Paul B Mahol

Signed-off-by: Paul B Mahol 
---
 libavfilter/Makefile |   2 +
 libavfilter/allfilters.c |   2 +
 libavfilter/f_graphmonitor.c | 417 +++
 3 files changed, 421 insertions(+)
 create mode 100644 libavfilter/f_graphmonitor.c

diff --git a/libavfilter/Makefile b/libavfilter/Makefile
index a98c64b7ce..c35cd8f422 100644
--- a/libavfilter/Makefile
+++ b/libavfilter/Makefile
@@ -240,6 +240,7 @@ OBJS-$(CONFIG_FSPP_FILTER)   += vf_fspp.o
 OBJS-$(CONFIG_GBLUR_FILTER)  += vf_gblur.o
 OBJS-$(CONFIG_GEQ_FILTER)+= vf_geq.o
 OBJS-$(CONFIG_GRADFUN_FILTER)+= vf_gradfun.o
+OBJS-$(CONFIG_GRAPHMONITOR_FILTER)   += f_graphmonitor.o
 OBJS-$(CONFIG_GREYEDGE_FILTER)   += vf_colorconstancy.o
 OBJS-$(CONFIG_HALDCLUT_FILTER)   += vf_lut3d.o framesync.o
 OBJS-$(CONFIG_HFLIP_FILTER)  += vf_hflip.o
@@ -437,6 +438,7 @@ OBJS-$(CONFIG_NULLSINK_FILTER)   += 
vsink_nullsink.o
 # multimedia filters
 OBJS-$(CONFIG_ABITSCOPE_FILTER)  += avf_abitscope.o
 OBJS-$(CONFIG_ADRAWGRAPH_FILTER) += f_drawgraph.o
+OBJS-$(CONFIG_AGRAPHMONITOR_FILTER)  += f_graphmonitor.o
 OBJS-$(CONFIG_AHISTOGRAM_FILTER) += avf_ahistogram.o
 OBJS-$(CONFIG_APHASEMETER_FILTER)+= avf_aphasemeter.o
 OBJS-$(CONFIG_AVECTORSCOPE_FILTER)   += avf_avectorscope.o
diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
index b2cb58fc38..d5a211bda5 100644
--- a/libavfilter/allfilters.c
+++ b/libavfilter/allfilters.c
@@ -227,6 +227,7 @@ extern AVFilter ff_vf_fspp;
 extern AVFilter ff_vf_gblur;
 extern AVFilter ff_vf_geq;
 extern AVFilter ff_vf_gradfun;
+extern AVFilter ff_vf_graphmonitor;
 extern AVFilter ff_vf_greyedge;
 extern AVFilter ff_vf_haldclut;
 extern AVFilter ff_vf_hflip;
@@ -418,6 +419,7 @@ extern AVFilter ff_vsink_nullsink;
 /* multimedia filters */
 extern AVFilter ff_avf_abitscope;
 extern AVFilter ff_avf_adrawgraph;
+extern AVFilter ff_avf_agraphmonitor;
 extern AVFilter ff_avf_ahistogram;
 extern AVFilter ff_avf_aphasemeter;
 extern AVFilter ff_avf_avectorscope;
diff --git a/libavfilter/f_graphmonitor.c b/libavfilter/f_graphmonitor.c
new file mode 100644
index 00..14f1670a3e
--- /dev/null
+++ b/libavfilter/f_graphmonitor.c
@@ -0,0 +1,417 @@
+/*
+ * Copyright (c) 2018 Paul B Mahol
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include "float.h"
+
+#include "libavutil/pixdesc.h"
+#include "libavutil/eval.h"
+#include "libavutil/intreadwrite.h"
+#include "libavutil/opt.h"
+#include "libavutil/timestamp.h"
+#include "libavutil/xga_font_data.h"
+#include "avfilter.h"
+#include "filters.h"
+#include "formats.h"
+#include "internal.h"
+#include "video.h"
+
+typedef struct GraphMonitorContext {
+const AVClass *class;
+
+int w, h;
+float opacity;
+int mode;
+int flags;
+AVRational frame_rate;
+
+int64_t pts;
+uint8_t white[4];
+uint8_t yellow[4];
+uint8_t red[4];
+uint8_t green[4];
+uint8_t bg[4];
+} GraphMonitorContext;
+
+enum {
+MODE_QUEUE = 1 << 0,
+MODE_FCIN  = 1 << 1,
+MODE_FCOUT = 1 << 2,
+MODE_PTS   = 1 << 3,
+MODE_TIME  = 1 << 4,
+MODE_TB= 1 << 5,
+MODE_FMT   = 1 << 6,
+MODE_SIZE  = 1 << 7,
+MODE_RATE  = 1 << 8,
+};
+
+#define OFFSET(x) offsetof(GraphMonitorContext, x)
+#define VF AV_OPT_FLAG_VIDEO_PARAM|AV_OPT_FLAG_FILTERING_PARAM
+
+static const AVOption graphmonitor_options[] = {
+{ "size", "set monitor size", OFFSET(w), AV_OPT_TYPE_IMAGE_SIZE, 
{.str="hd720"}, 0, 0, VF },
+{ "s","set monitor size", OFFSET(w), AV_OPT_TYPE_IMAGE_SIZE, 
{.str="hd720"}, 0, 0, VF },
+{ "opacity", "set video opacity", OFFSET(opacity), AV_OPT_TYPE_FLOAT, 
{.dbl=.9}, 0, 1, VF },
+{ "o",   "set video opacity", OFFSET(opacity), AV_OPT_TYPE_FLOAT, 
{.dbl=.9}, 0, 1, VF },
+{ "mode", "set mode", OFFSET(mode), AV_OPT_TYPE_INT, {.i64=0}, 0, 1, VF, 
"mode" },
+{ "m","set mode", OFFSET(mode), AV_OPT_TYPE_INT, {.i64=0}, 0, 1, VF, 
"mode" },
+{ "full", NULL, 0, AV_OPT_TYPE_CONST, {.i64=0},   0, 0, VF, "mode" 
},
+{ "compact",  NULL, 0, AV_OPT_TYPE_CONST, {.i64=1},   0, 0, VF, "mode" 
},
+

Re: [FFmpeg-devel] [PATCH 1/2] libavutil: Undeprecate the AVFrame reordered_opaque field

2018-10-29 Thread Martin Storsjö


On Mon, 29 Oct 2018, Derek Buitenhuis wrote:


On 29/10/2018 14:10, Martin Storsjö wrote:

I don't understand why this is being used in favour of a proper
pointer field? An integer field is just ascting to be misused.
Even the doxygen is really sketchy on it.


It's essentially meant to be used as union { ptr; int64_t } assuming you
don't have pointers larger than 64 bits.


It's not a union in the API, and I'm pretty sure that it violates the C spec
to use a unions to get an integer out of a pointer, shove it into an int64_t,
and then get it back out, and chnage it back via union. Especially for
32-bit pointers. It encourages terrible code.

I just don't think we should revive this as-is purely for convenience.


I also don't understand why this is at the AVCodecContext level
and not packet/frame?


It is on the frame level, but not in the packet struct (probably for
historical reasons) - instead of in the packet, it's in AVCodecContext.
For decoding, you set the value in AVCodecContext before feeding packets
to it, and get the corresponding value reordered into the output AVFrame.
If things were to be redone from scratch, moving it into AVPacket would
probably make more sense, but there's not much point in doing that right
now.


I mean, this is pretty gross, and non-obvious as far as I'm concerned.
Modifying the AVCodecContext on every call is just... eugh.


At some point, the doxygen got markers saying this mechanism was
deprecated and one should use the new pkt_pts instead. Before that,
reordered_opaque was mainly used for getting reordered pts as there
was no other mechanism for it.

But even with the proper pkt_pts field, having a generic opaque field that
travels along with the reordering is useful, which is why the deprecation
doxygen comments were removed in ad1ee5fa7. But that commit just missed to
remove one of the doxygen deprecation.


I agree it's very useful, and something we should have, but not that we should
revive/use this partiular field... it's nasty.


Sorry, I think you've misunderstood this patch altogether.

It's not about reviving this field or not, it's all in full use 
already. It was never deprecated with any active plan to remove it, the 
only steps were a few doxygen comments, never any attributes that would 
actually prompt action.


And a few years later someone noticed that these doxygen comments didn't 
match up with reality, and it was decided (with no objections on either 
project) that these really shouldn't be deprecated as it is the only 
actual mechanism we have for doing exactly this.


It's just that the undeprecation commit, ad1ee5fa7, missed one field. And 
the one I'm removing the stray deprecation comment from, is the very 
properly placed one in AVFrame non the less.


// Martin
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] 2 alternative ways to check in vp9 decode_tiles() if there is data remaining

2018-10-29 Thread Michael Niedermayer

On Sat, Oct 20, 2018 at 12:42:35PM +0200, Michael Niedermayer wrote:
> Hi
> 
> 2 alternative patchsets are attached to fix $SUBJ
> 
> The 2 alternatives should behave similar.
> 
> The first adds a function to check if the next range coder symbol read would
> trigger the end of input case.
> We then error out before reading in case the read would trigger this case
> 
> The second sets a flag if the end of input case triggered and subsequently
> errors out
> 
> The second case should be slower as it requires additional checks in inner
> loops, but i was asked to implement this with a flag, so i implemented both
> ways.
> 
> Which version, if any, should i apply ?

this also fixes 
9775/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_VP9_fuzzer-5643845344690176
ill apply the one that avoids checks in the inner loop.
If people prefer the other iam happy to revert it and replace it by the
other solution. But i dont want to leave the issue open 

Thanks
[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

"Nothing to hide" only works if the folks in power share the values of
you and everyone you know entirely and always will -- Tom Scott



signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/2] libavutil: Undeprecate the AVFrame reordered_opaque field

2018-10-29 Thread Derek Buitenhuis

On 29/10/2018 20:10, Martin Storsjö wrote:
> Sorry, I think you've misunderstood this patch altogether.
> 
> It's not about reviving this field or not, it's all in full use
> already. It was never deprecated with any active plan to remove it, the
> only steps were a few doxygen comments, never any attributes that would
> actually prompt action.
> 
> And a few years later someone noticed that these doxygen comments didn't
> match up with reality, and it was decided (with no objections on either
> project) that these really shouldn't be deprecated as it is the only
> actual mechanism we have for doing exactly this.
> 
> It's just that the undeprecation commit, ad1ee5fa7, missed one field. And
> the one I'm removing the stray deprecation comment from, is the very
> properly placed one in AVFrame non the less.

Well, you're adding new code that uses it... which, in my mind is akin to
encouraging its use. But, yes, you're not technical undeprecating.

I'm kinda tempted to send a patch to actually deprecate it again, and
replace it with something less terrible. You've kinda shone a flashlight
on a particularily terrible (IMO) API, is all. Looking back at the list
where this was undeprecated, no real reasoning was given for keeping it
around in this form, AFAICT, and nobody asked either. It was kept because
it was effort to replace it, I guess.

I won't block the patch set, but will note that I object to continued
existence of such a terrible API, and adding new stuff that uses it.

- Derek
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 2/2] libx264: Pass the reordered_opaque field through the encoder

2018-10-29 Thread Derek Buitenhuis

On 25/10/2018 13:58, Martin Storsjö wrote:
> +x4->nb_reordered_opaque = x264_encoder_maximum_delayed_frames(x4->enc) + 
> 1;

Is it possible this changes when the encoder is reconfigured (e.g. to 
interlaced)?

- Derek
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/5] lavc/qsvenc: add forced_idr opiton

2018-10-29 Thread Rogozhkin, Dmitry V

On Fri, 2018-10-26 at 13:46 +0200, Moritz Barsnick wrote:
> On Thu, Oct 25, 2018 at 20:36:07 +0800, Zhong Li wrote:
> > +{ "forced_idr", "Forcing I frames as IDR
> > frames", OFFSET(qsv.forced_idr), AV_OPT_TYPE_INT, {
> > .i64 = -1 }, -1,  1, VE }, \
> 
> ffmpeg uses imperative (mostly): "Force I frames as IDR frames".
> 

Should not the option be named 'force_idr' as well? It makes better
sense to me in that way...

> Moritz
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/5] lavc/qsvenc: add forced_idr opiton

2018-10-29 Thread Derek Buitenhuis

On 29/10/2018 20:51, Rogozhkin, Dmitry V wrote:
> Should not the option be named 'force_idr' as well? It makes better
> sense to me in that way...

That would be inconsistent with the rest of the options for various encoders
in FFmpeg, all named forced_idr.

- Derek
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/5] lavc/qsvenc: add forced_idr opiton

2018-10-29 Thread Mark Thompson

On 25/10/18 13:36, Zhong Li wrote:
> This option can be used to repect original input I/IDR frame type.
> 
> Signed-off-by: Zhong Li 
> ---
>  libavcodec/qsvenc.c | 7 +++
>  libavcodec/qsvenc.h | 2 ++
>  2 files changed, 9 insertions(+)
> 
> diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c
> index 948751d..e534dcf 100644
> --- a/libavcodec/qsvenc.c
> +++ b/libavcodec/qsvenc.c
> @@ -1192,6 +1192,13 @@ static int encode_frame(AVCodecContext *avctx, 
> QSVEncContext *q,
>  if (qsv_frame) {
>  surf = &qsv_frame->surface;
>  enc_ctrl = &qsv_frame->enc_ctrl;
> +
> +if (q->forced_idr >= 0 && frame->pict_type == AV_PICTURE_TYPE_I) {
> +enc_ctrl->FrameType = MFX_FRAMETYPE_I | MFX_FRAMETYPE_REF;
> +if (q->forced_idr || frame->key_frame)
> +enc_ctrl->FrameType |= MFX_FRAMETYPE_IDR;
> +} else
> +enc_ctrl->FrameType = MFX_FRAMETYPE_UNKNOWN;
>  }
>  
>  ret = av_new_packet(&new_pkt, q->packet_size);
> diff --git a/libavcodec/qsvenc.h b/libavcodec/qsvenc.h
> index 055b4a6..1f97f77 100644
> --- a/libavcodec/qsvenc.h
> +++ b/libavcodec/qsvenc.h
> @@ -87,6 +87,7 @@
>  { "adaptive_i", "Adaptive I-frame placement", 
> OFFSET(qsv.adaptive_i), AV_OPT_TYPE_INT, { .i64 = -1 }, -1,  1, 
> VE }, \
>  { "adaptive_b", "Adaptive B-frame placement", 
> OFFSET(qsv.adaptive_b), AV_OPT_TYPE_INT, { .i64 = -1 }, -1,  1, 
> VE }, \
>  { "b_strategy", "Strategy to choose between I/P/B-frames", 
> OFFSET(qsv.b_strategy),AV_OPT_TYPE_INT, { .i64 = -1 }, -1,  1, VE 
> }, \
> +{ "forced_idr", "Forcing I frames as IDR frames", 
> OFFSET(qsv.forced_idr), AV_OPT_TYPE_INT, { .i64 = -1 }, -1,  1, 
> VE }, \
>  
>  typedef int SetEncodeCtrlCB (AVCodecContext *avctx,
>   const AVFrame *frame, mfxEncodeCtrl* enc_ctrl);
> @@ -168,6 +169,7 @@ typedef struct QSVEncContext {
>  #endif
>  char *load_plugins;
>  SetEncodeCtrlCB *set_encode_ctrl_cb;
> +int forced_idr;
>  } QSVEncContext;
>  
>  int ff_qsv_enc_init(AVCodecContext *avctx, QSVEncContext *q);
> 

This seems confusing, because it doesn't match what forced_idr does in any 
other encoder.

Checking of pict_type for AV_PICTURE_TYPE_I in order to get a key frame (of 
whatever kind) is always enabled if supported (many encoders).  The 
"forced_idr" option to H.26[45] encoders (libx264, libx265) then forces that to 
be an IDR frame, not just an I frame.

- Mark
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 2/2] libx264: Pass the reordered_opaque field through the encoder

2018-10-29 Thread Martin Storsjö


On Mon, 29 Oct 2018, Derek Buitenhuis wrote:


On 25/10/2018 13:58, Martin Storsjö wrote:

+x4->nb_reordered_opaque = x264_encoder_maximum_delayed_frames(x4->enc) + 1;


Is it possible this changes when the encoder is reconfigured (e.g. to 
interlaced)?


Good point. I'm sure it's possible that it changes, if reconfiguring.

As I guess there can be old frames in flight, the only safe option is to 
enlarge, not to shrink it. But in case a realloc moves the array, the old 
pointers end up pretty useless.


Tricky, I guess I'll have to think about it to see if I can come up with 
something which isn't completely terrible.


// Martin
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/5] lavc/qsvenc: add forced_idr opiton

2018-10-29 Thread Rogozhkin, Dmitry V

On Thu, 2018-10-25 at 20:36 +0800, Zhong Li wrote:
> This option can be used to repect original input I/IDR frame type.
> 
> Signed-off-by: Zhong Li 
> ---
>  libavcodec/qsvenc.c | 7 +++
>  libavcodec/qsvenc.h | 2 ++
>  2 files changed, 9 insertions(+)
> 
> diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c
> index 948751d..e534dcf 100644
> --- a/libavcodec/qsvenc.c
> +++ b/libavcodec/qsvenc.c
> @@ -1192,6 +1192,13 @@ static int encode_frame(AVCodecContext *avctx,
> QSVEncContext *q,
>  if (qsv_frame) {
>  surf = &qsv_frame->surface;
>  enc_ctrl = &qsv_frame->enc_ctrl;
> +
> +if (q->forced_idr >= 0 && frame->pict_type ==
> AV_PICTURE_TYPE_I) {
> +enc_ctrl->FrameType = MFX_FRAMETYPE_I |
> MFX_FRAMETYPE_REF;
> +if (q->forced_idr || frame->key_frame)
> +enc_ctrl->FrameType |= MFX_FRAMETYPE_IDR;

Hm. There is another option "-force_key_frames" which looks unhandled
here. At least I don't understand "|| frame->key_frame"...


> +} else
> +enc_ctrl->FrameType = MFX_FRAMETYPE_UNKNOWN;

"else" block don't make much sense to me. You eventually already had
enc_ctrl structure passed to the encoder. Thus, it should be
initialized to default (already). And you don't make anything
specific/new in the "else". From my perspective "else" just obscures
the code and should be dropped.
>  }
>  
>  ret = av_new_packet(&new_pkt, q->packet_size);
> diff --git a/libavcodec/qsvenc.h b/libavcodec/qsvenc.h
> index 055b4a6..1f97f77 100644
> --- a/libavcodec/qsvenc.h
> +++ b/libavcodec/qsvenc.h
> @@ -87,6 +87,7 @@
>  { "adaptive_i", "Adaptive I-frame
> placement", OFFSET(qsv.adaptive_i), AV_OPT_TYPE_INT,
> { .i64 = -1 }, -1,  1, VE }, \
>  { "adaptive_b", "Adaptive B-frame
> placement", OFFSET(qsv.adaptive_b), AV_OPT_TYPE_INT,
> { .i64 = -1 }, -1,  1, VE }, \
>  { "b_strategy", "Strategy to choose between I/P/B-frames",
> OFFSET(qsv.b_strategy),AV_OPT_TYPE_INT, { .i64 = -1 },
> -1,  1, VE }, \
> +{ "forced_idr", "Forcing I frames as IDR
> frames", OFFSET(qsv.forced_idr), AV_OPT_TYPE_INT, { .i64
> = -1 }, -1,  1, VE }, \

As pointed out in other mail, I think this should be "force_idr" option
and "Force I frames as IDR frames" as an option description. Secondly,
why there are 3 values accepted: -1, 0, 1? I can understand 1 as enable
the feature and 0 as don't enable, but what is -1? Also, how the option
correlates with "-force_key_frames"?

From this perspective shouldn't the code be:

{ "force_idr", "Force I frames as IDR
frames", OFFSET(qsv.force_idr), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 1, VE
},

if (frame->pict_type == AV_PICTURE_TYPE_I && (frame->key_frame || q-
>force_idr)) {
   enc_ctrl->FrameType = MFX_FRAMETYPE_I |
MFX_FRAMETYPE_REF;
   if (q->force_idr)
  enc_ctrl->FrameType |= MFX_FRAMETYPE_IDR;
}

This assumes that frame->key_frame corresponds to "-force_key_frames"
in which I am not quite sure...

>  
>  typedef int SetEncodeCtrlCB (AVCodecContext *avctx,
>   const AVFrame *frame, mfxEncodeCtrl*
> enc_ctrl);
> @@ -168,6 +169,7 @@ typedef struct QSVEncContext {
>  #endif
>  char *load_plugins;
>  SetEncodeCtrlCB *set_encode_ctrl_cb;
> +int forced_idr;

int force_idr;

if agreed on the above...

>  } QSVEncContext;
>  
>  int ff_qsv_enc_init(AVCodecContext *avctx, QSVEncContext *q);
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] tests/api-h264-slice-test: fread returns size_t, not ssize_t.

2018-10-29 Thread Carl Eugen Hoyos

Hi!

Attached untested patch is supposed to fix ticket #7521.

Please comment, Carl Eugen
From ad5a9de93a8cfe261636993532246befddea984d Mon Sep 17 00:00:00 2001
From: Carl Eugen Hoyos 
Date: Mon, 29 Oct 2018 22:11:54 +0100
Subject: [PATCH] tests/api-h264-slice-test: fread returns size_t, not
 ssize_t.

Fixes ticket #7521.
---
 tests/api/api-h264-slice-test.c |6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/api/api-h264-slice-test.c b/tests/api/api-h264-slice-test.c
index 57e7dc7..9ca30dc 100644
--- a/tests/api/api-h264-slice-test.c
+++ b/tests/api/api-h264-slice-test.c
@@ -180,15 +180,15 @@ int main(int argc, char **argv)
 
 while(1) {
 uint16_t size = 0;
-ssize_t ret = fread(&size, 1, sizeof(uint16_t), fd);
-if (ret < 0) {
+size_t ret = fread(&size, 1, sizeof(uint16_t), fd);
+if (!ret) {
 perror("Couldn't read size");
 exit(1);
 } else if (ret != sizeof(uint16_t))
 break;
 size = ntohs(size);
 ret = fread(p, 1, size, fd);
-if (ret < 0 || ret != size) {
+if (ret != size) {
 perror("Couldn't read data");
 exit(1);
 }
-- 
1.7.10.4

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v2] fate/api-h264-slice-test: use cleaner error handling

2018-10-29 Thread Carl Eugen Hoyos

2018-10-29 15:49 GMT+01:00, jos...@ob-encoder.com :

>  while(1) {
>  uint16_t size = 0;
> -ssize_t ret = fread(&size, 1, sizeof(uint16_t), fd);
> -if (ret < 0) {
> -perror("Couldn't read size");
> -exit(1);
> -} else if (ret != sizeof(uint16_t))
> +size_t ret = fread(&size, 1, sizeof(uint16_t), file);
> +if (ret != sizeof(uint16_t))

I didn't see this originally because it is a little to well hidden.

Please split this out and mention ticket #7521.

Thank you, Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v2 1/1] avcodec/mf: implemented Media Foundation wrapper

2018-10-29 Thread Carl Eugen Hoyos

2018-10-29 10:27 GMT+01:00, Paweł Wegner :

> +  --enable-mf  enable decoding via MediaFoundation [no]

Should be auto-detected.

Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v2 1/1] avcodec/mf: implemented Media Foundation wrapper

2018-10-29 Thread Paul B Mahol

On 10/29/18, Carl Eugen Hoyos  wrote:
> 2018-10-29 10:27 GMT+01:00, Paweł Wegner :
>
>> +  --enable-mf  enable decoding via MediaFoundation [no]
>
> Should be auto-detected.

Why? I do not want this to be auto-enabled on my Windows builds.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/2] lavc/amfenc: moving amf common code (library and context) to lavu/hwcontext_amf from amfenc to be reused in other amf components

2018-10-29 Thread Mark Thompson

On 29/10/18 11:45, Alexander Kravchenko wrote:
> Hi, Mark.
> Thanks for review.
> Could you please check the following comments/questions?
> 
>> +
>>> +static const AVClass amflib_class = {
>>> +.class_name = "amf",
>>> +.item_name = av_default_item_name,
>>> +.version = LIBAVUTIL_VERSION_INT,
>>> +};
>>
>> This class shouldn't be needed - the right class to use is the one in the
>> AVHWDeviceContext, you should be able to pass it to the right place via
>> your AMFDeviceContextPrivate structure.
>>
>>> +
>>> +typedef struct AMFLibraryContext {
>>> +const AVClass  *avclass;
>>> +} AMFLibraryContext;
>>> +
>>> +static AMFLibraryContext amflib_context =
>>> +{
>>> +.avclass = &amflib_class,
>>> +};
>>
>> This structure is just a dummy for the class?  Use the AVHWDeviceContext.
>>
>>> +
>>> +typedef struct AmfTraceWriter {
>>> +const AMFTraceWriterVtbl*vtbl;
>>> +void*avcl;
>>> +} AmfTraceWriter;
>>> +
>>> +static void AMF_CDECL_CALL AMFTraceWriter_Write(AMFTraceWriter *pThis,
>>
>> It would be sensible to take the opportunity to fix the function name to
>> conform to ffmpeg style.
>>
>>> +const wchar_t *scope, const wchar_t *message)
>>> +{
>>> +AmfTraceWriter *tracer = (AmfTraceWriter*)pThis;
>>> +av_log(tracer->avcl, AV_LOG_DEBUG, "%ls: %ls", scope, message);
>>> +}
>>> +
>>> +static void AMF_CDECL_CALL AMFTraceWriter_Flush(AMFTraceWriter *pThis)
>>> +{
>>> +}
>>> +
>>> +static const AMFTraceWriterVtbl tracer_vtbl =
>>> +{
>>> +.Write = AMFTraceWriter_Write,
>>> +.Flush = AMFTraceWriter_Flush,
>>
>> Is this function really required to exist, given that it doesn't do
>> anything?
>>
>>> +};
>>> +
>>> +static const AmfTraceWriter amf_trace_writer =
>>> +{
>>> +.vtbl = &tracer_vtbl,
>>> +.avcl = &amflib_context,
>>> +};
>>
>> This should probably be inside the AMFDeviceContextPrivate, so that it can
>> point to the right context structure.
>>
>> This is the question.
> AMF Library has global Trace settings, not per AMFContext object.

So that global state is inside the AMF library?

How does that interact with the fact that you reload it and reconfigure it 
every time it gets loaded - if one thread calls amf_device_create() while 
another one is inside the encoder and generating log output (say), what happens?

(I also note that it presumably means the current AMF encoder code will crash 
if you have two encoders with nested lifetimes - the second encoder will 
overwrite the global state, and once it finishes and gets freed the global 
trace output from the first encoder will have an invalid class pointer.)

> My intension was to create global AMF lib class and Tracer object which
> refers to it as class parameter in av_log call.
> It is required in scenario when multiple hwcontext_amf are created during
> application lifecycle.
> if this way is ok, should I add comments to code describes this? or is
> there another way to have global object to handle this?
Global state in libraries really isn't ok.  I think in response to that I would 
force it to be completely off by default, maybe switch it on if the user passes 
some special named option to amf_device_create()?

> AMFTraceWriterVtbl.Flush - I am not sure that it can be set as null
> pointer, I could double check with AMD developers.

It doesn't really matter; just the empty function didn't seem useful.

>>>  #include 
>>>  #include 
>>
>> Kindof unrelated, but is there any reason why both of these are in the
>> header rather than in the per-codec files?
>>
>>
> Component management code is the same for all encoder components.
> The only encoder id defines is used for component creation here.

Yep, missed that.  Sure.

>>
>> The log_to_dbg option is orphaned by this change.  Is it worth keeping?
>> (If you want to keep it then maybe it could be a named option to
>> av_hwdevice_ctx_create() -> amf_device_create().)
>>
>> log_to_dbg is removed from here, because this setting is global for AMF
> library, not per component(encoder) or per AMFContext(hwcontext_amf object)
> 
> Probably I could implement some global AMF lib options which configures
> tracer more precisely in general

Comment above applies, I think.

- Mark
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/5] lavc/qsvenc: add forced_idr opiton

2018-10-29 Thread Rogozhkin, Dmitry V

On Mon, 2018-10-29 at 20:54 +, Derek Buitenhuis wrote:
> On 29/10/2018 20:51, Rogozhkin, Dmitry V wrote:
> > Should not the option be named 'force_idr' as well? It makes better
> > sense to me in that way...
> 
> That would be inconsistent with the rest of the options for various
> encoders
> in FFmpeg, all named forced_idr.

Ok. Better to be consistent with the option names across components. I
have overlooked this in the help though I have tried to check.

> 
> - Derek
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/5] lavc/qsvenc: add forced_idr opiton

2018-10-29 Thread Rogozhkin, Dmitry V

On Mon, 2018-10-29 at 21:06 +, Mark Thompson wrote:
> On 25/10/18 13:36, Zhong Li wrote:
> > This option can be used to repect original input I/IDR frame type.
> > 
> > Signed-off-by: Zhong Li 
> > ---
> >  libavcodec/qsvenc.c | 7 +++
> >  libavcodec/qsvenc.h | 2 ++
> >  2 files changed, 9 insertions(+)
> > 
> > diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c
> > index 948751d..e534dcf 100644
> > --- a/libavcodec/qsvenc.c
> > +++ b/libavcodec/qsvenc.c
> > @@ -1192,6 +1192,13 @@ static int encode_frame(AVCodecContext
> > *avctx, QSVEncContext *q,
> >  if (qsv_frame) {
> >  surf = &qsv_frame->surface;
> >  enc_ctrl = &qsv_frame->enc_ctrl;
> > +
> > +if (q->forced_idr >= 0 && frame->pict_type ==
> > AV_PICTURE_TYPE_I) {
> > +enc_ctrl->FrameType = MFX_FRAMETYPE_I |
> > MFX_FRAMETYPE_REF;
> > +if (q->forced_idr || frame->key_frame)
> > +enc_ctrl->FrameType |= MFX_FRAMETYPE_IDR;
> > +} else
> > +enc_ctrl->FrameType = MFX_FRAMETYPE_UNKNOWN;
> >  }
> >  
> >  ret = av_new_packet(&new_pkt, q->packet_size);
> > diff --git a/libavcodec/qsvenc.h b/libavcodec/qsvenc.h
> > index 055b4a6..1f97f77 100644
> > --- a/libavcodec/qsvenc.h
> > +++ b/libavcodec/qsvenc.h
> > @@ -87,6 +87,7 @@
> >  { "adaptive_i", "Adaptive I-frame
> > placement", OFFSET(qsv.adaptive_i), AV_OPT_TYPE_INT
> > , { .i64 = -1 }, -1,  1, VE }, \
> >  { "adaptive_b", "Adaptive B-frame
> > placement", OFFSET(qsv.adaptive_b), AV_OPT_TYPE_INT
> > , { .i64 = -1 }, -1,  1, VE }, \
> >  { "b_strategy", "Strategy to choose between I/P/B-frames",
> > OFFSET(qsv.b_strategy),AV_OPT_TYPE_INT, { .i64 = -1 },
> > -1,  1, VE }, \
> > +{ "forced_idr", "Forcing I frames as IDR
> > frames", OFFSET(qsv.forced_idr), AV_OPT_TYPE_INT, {
> > .i64 = -1 }, -1,  1, VE }, \
> >  
> >  typedef int SetEncodeCtrlCB (AVCodecContext *avctx,
> >   const AVFrame *frame, mfxEncodeCtrl*
> > enc_ctrl);
> > @@ -168,6 +169,7 @@ typedef struct QSVEncContext {
> >  #endif
> >  char *load_plugins;
> >  SetEncodeCtrlCB *set_encode_ctrl_cb;
> > +int forced_idr;
> >  } QSVEncContext;
> >  
> >  int ff_qsv_enc_init(AVCodecContext *avctx, QSVEncContext *q);
> > 
> 
> This seems confusing, because it doesn't match what forced_idr does
> in any other encoder.
> 
> Checking of pict_type for AV_PICTURE_TYPE_I in order to get a key
> frame (of whatever kind) is always enabled if supported (many
> encoders).  The "forced_idr" option to H.26[45] encoders (libx264,
> libx265) then forces that to be an IDR frame, not just an I frame.

Is there an option to disable/override this behavior? Will "-
force_key_frames" do the trick?

> - Mark
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/5] lavc/qsvenc: add forced_idr opiton

2018-10-29 Thread Mark Thompson

On 29/10/18 21:29, Rogozhkin, Dmitry V wrote:
> On Mon, 2018-10-29 at 21:06 +, Mark Thompson wrote:
>> On 25/10/18 13:36, Zhong Li wrote:
>>> This option can be used to repect original input I/IDR frame type.
>>>
>>> Signed-off-by: Zhong Li 
>>> ---
>>>  libavcodec/qsvenc.c | 7 +++
>>>  libavcodec/qsvenc.h | 2 ++
>>>  2 files changed, 9 insertions(+)
>>>
>>> diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c
>>> index 948751d..e534dcf 100644
>>> --- a/libavcodec/qsvenc.c
>>> +++ b/libavcodec/qsvenc.c
>>> @@ -1192,6 +1192,13 @@ static int encode_frame(AVCodecContext
>>> *avctx, QSVEncContext *q,
>>>  if (qsv_frame) {
>>>  surf = &qsv_frame->surface;
>>>  enc_ctrl = &qsv_frame->enc_ctrl;
>>> +
>>> +if (q->forced_idr >= 0 && frame->pict_type ==
>>> AV_PICTURE_TYPE_I) {
>>> +enc_ctrl->FrameType = MFX_FRAMETYPE_I |
>>> MFX_FRAMETYPE_REF;
>>> +if (q->forced_idr || frame->key_frame)
>>> +enc_ctrl->FrameType |= MFX_FRAMETYPE_IDR;
>>> +} else
>>> +enc_ctrl->FrameType = MFX_FRAMETYPE_UNKNOWN;
>>>  }
>>>  
>>>  ret = av_new_packet(&new_pkt, q->packet_size);
>>> diff --git a/libavcodec/qsvenc.h b/libavcodec/qsvenc.h
>>> index 055b4a6..1f97f77 100644
>>> --- a/libavcodec/qsvenc.h
>>> +++ b/libavcodec/qsvenc.h
>>> @@ -87,6 +87,7 @@
>>>  { "adaptive_i", "Adaptive I-frame
>>> placement", OFFSET(qsv.adaptive_i), AV_OPT_TYPE_INT
>>> , { .i64 = -1 }, -1,  1, VE }, \
>>>  { "adaptive_b", "Adaptive B-frame
>>> placement", OFFSET(qsv.adaptive_b), AV_OPT_TYPE_INT
>>> , { .i64 = -1 }, -1,  1, VE }, \
>>>  { "b_strategy", "Strategy to choose between I/P/B-frames",
>>> OFFSET(qsv.b_strategy),AV_OPT_TYPE_INT, { .i64 = -1 },
>>> -1,  1, VE }, \
>>> +{ "forced_idr", "Forcing I frames as IDR
>>> frames", OFFSET(qsv.forced_idr), AV_OPT_TYPE_INT, {
>>> .i64 = -1 }, -1,  1, VE }, \
>>>  
>>>  typedef int SetEncodeCtrlCB (AVCodecContext *avctx,
>>>   const AVFrame *frame, mfxEncodeCtrl*
>>> enc_ctrl);
>>> @@ -168,6 +169,7 @@ typedef struct QSVEncContext {
>>>  #endif
>>>  char *load_plugins;
>>>  SetEncodeCtrlCB *set_encode_ctrl_cb;
>>> +int forced_idr;
>>>  } QSVEncContext;
>>>  
>>>  int ff_qsv_enc_init(AVCodecContext *avctx, QSVEncContext *q);
>>>
>>
>> This seems confusing, because it doesn't match what forced_idr does
>> in any other encoder.
>>
>> Checking of pict_type for AV_PICTURE_TYPE_I in order to get a key
>> frame (of whatever kind) is always enabled if supported (many
>> encoders).  The "forced_idr" option to H.26[45] encoders (libx264,
>> libx265) then forces that to be an IDR frame, not just an I frame.
> 
> Is there an option to disable/override this behavior? Will "-
> force_key_frames" do the trick?

Not in libavcodec; encoders always looks at pict_type (e.g. 
).

"-force_key_frames" is an option to the ffmpeg utility which sets pict_type 
dependent on some expression, making use of the above behaviour.

- Mark
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/5] lavc/qsvenc: add forced_idr opiton

2018-10-29 Thread Rogozhkin, Dmitry V

On Mon, 2018-10-29 at 21:34 +, Mark Thompson wrote:
> On 29/10/18 21:29, Rogozhkin, Dmitry V wrote:
> > On Mon, 2018-10-29 at 21:06 +, Mark Thompson wrote:
> > > On 25/10/18 13:36, Zhong Li wrote:
> > > > This option can be used to repect original input I/IDR frame
> > > > type.
> > > > 
> > > > Signed-off-by: Zhong Li 
> > > > ---
> > > >  libavcodec/qsvenc.c | 7 +++
> > > >  libavcodec/qsvenc.h | 2 ++
> > > >  2 files changed, 9 insertions(+)
> > > > 
> > > > diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c
> > > > index 948751d..e534dcf 100644
> > > > --- a/libavcodec/qsvenc.c
> > > > +++ b/libavcodec/qsvenc.c
> > > > @@ -1192,6 +1192,13 @@ static int encode_frame(AVCodecContext
> > > > *avctx, QSVEncContext *q,
> > > >  if (qsv_frame) {
> > > >  surf = &qsv_frame->surface;
> > > >  enc_ctrl = &qsv_frame->enc_ctrl;
> > > > +
> > > > +if (q->forced_idr >= 0 && frame->pict_type ==
> > > > AV_PICTURE_TYPE_I) {
> > > > +enc_ctrl->FrameType = MFX_FRAMETYPE_I |
> > > > MFX_FRAMETYPE_REF;
> > > > +if (q->forced_idr || frame->key_frame)
> > > > +enc_ctrl->FrameType |= MFX_FRAMETYPE_IDR;
> > > > +} else
> > > > +enc_ctrl->FrameType = MFX_FRAMETYPE_UNKNOWN;
> > > >  }
> > > >  
> > > >  ret = av_new_packet(&new_pkt, q->packet_size);
> > > > diff --git a/libavcodec/qsvenc.h b/libavcodec/qsvenc.h
> > > > index 055b4a6..1f97f77 100644
> > > > --- a/libavcodec/qsvenc.h
> > > > +++ b/libavcodec/qsvenc.h
> > > > @@ -87,6 +87,7 @@
> > > >  { "adaptive_i", "Adaptive I-frame
> > > > placement", OFFSET(qsv.adaptive_i), AV_OPT_TYPE
> > > > _INT
> > > > , { .i64 = -1 }, -1,  1, VE
> > > > }, \
> > > >  { "adaptive_b", "Adaptive B-frame
> > > > placement", OFFSET(qsv.adaptive_b), AV_OPT_TYPE
> > > > _INT
> > > > , { .i64 = -1 }, -1,  1, VE
> > > > }, \
> > > >  { "b_strategy", "Strategy to choose between I/P/B-frames",
> > > > OFFSET(qsv.b_strategy),AV_OPT_TYPE_INT, { .i64 = -1 },
> > > > -1,  1, VE }, \
> > > > +{ "forced_idr", "Forcing I frames as IDR
> > > > frames", OFFSET(qsv.forced_idr), AV_OPT_TYPE_INT, {
> > > > .i64 = -1 }, -1,  1, VE }, \
> > > >  
> > > >  typedef int SetEncodeCtrlCB (AVCodecContext *avctx,
> > > >   const AVFrame *frame,
> > > > mfxEncodeCtrl*
> > > > enc_ctrl);
> > > > @@ -168,6 +169,7 @@ typedef struct QSVEncContext {
> > > >  #endif
> > > >  char *load_plugins;
> > > >  SetEncodeCtrlCB *set_encode_ctrl_cb;
> > > > +int forced_idr;
> > > >  } QSVEncContext;
> > > >  
> > > >  int ff_qsv_enc_init(AVCodecContext *avctx, QSVEncContext *q);
> > > > 
> > > 
> > > This seems confusing, because it doesn't match what forced_idr
> > > does
> > > in any other encoder.
> > > 
> > > Checking of pict_type for AV_PICTURE_TYPE_I in order to get a key
> > > frame (of whatever kind) is always enabled if supported (many
> > > encoders).  The "forced_idr" option to H.26[45] encoders
> > > (libx264,
> > > libx265) then forces that to be an IDR frame, not just an I
> > > frame.
> > 
> > Is there an option to disable/override this behavior? Will "-
> > force_key_frames" do the trick?
> 
> Not in libavcodec; encoders always looks at pict_type (e.g.
>  h=d6367bf557953ec4e91d057d551848b3b2ba9d36;hb=HEAD#l300>).

Where gop structure is being set in SW encoders? I mean, is pict_type
set by infrastructure according to -g and -bf options + some
information on the incoming stream (passing thru I frames probably?) or
pict_type is in a way recommendation to follow, but general gop
structure is calculated/maintained inside the encoder?

> 
> "-force_key_frames" is an option to the ffmpeg utility which sets
> pict_type dependent on some expression, making use of the above
> behaviour.
> 
> - Mark
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/2] lavc/amfenc: moving amf common code (library and context) to lavu/hwcontext_amf from amfenc to be reused in other amf components

2018-10-29 Thread Alexander Kravchenko

Hi Mark,
see my comments bellow.

вт, 30 окт. 2018 г. в 0:23, Mark Thompson :

> On 29/10/18 11:45, Alexander Kravchenko wrote:
> > Hi, Mark.
> > Thanks for review.
> > Could you please check the following comments/questions?
> >
> >> +
> >>> +static const AVClass amflib_class = {
> >>> +.class_name = "amf",
> >>> +.item_name = av_default_item_name,
> >>> +.version = LIBAVUTIL_VERSION_INT,
> >>> +};
> >>
> >> This class shouldn't be needed - the right class to use is the one in
> the
> >> AVHWDeviceContext, you should be able to pass it to the right place via
> >> your AMFDeviceContextPrivate structure.
> >>
> >>> +
> >>> +typedef struct AMFLibraryContext {
> >>> +const AVClass  *avclass;
> >>> +} AMFLibraryContext;
> >>> +
> >>> +static AMFLibraryContext amflib_context =
> >>> +{
> >>> +.avclass = &amflib_class,
> >>> +};
> >>
> >> This structure is just a dummy for the class?  Use the
> AVHWDeviceContext.
> >>
> >>> +
> >>> +typedef struct AmfTraceWriter {
> >>> +const AMFTraceWriterVtbl*vtbl;
> >>> +void*avcl;
> >>> +} AmfTraceWriter;
> >>> +
> >>> +static void AMF_CDECL_CALL AMFTraceWriter_Write(AMFTraceWriter *pThis,
> >>
> >> It would be sensible to take the opportunity to fix the function name to
> >> conform to ffmpeg style.
> >>
> >>> +const wchar_t *scope, const wchar_t *message)
> >>> +{
> >>> +AmfTraceWriter *tracer = (AmfTraceWriter*)pThis;
> >>> +av_log(tracer->avcl, AV_LOG_DEBUG, "%ls: %ls", scope, message);
> >>> +}
> >>> +
> >>> +static void AMF_CDECL_CALL AMFTraceWriter_Flush(AMFTraceWriter *pThis)
> >>> +{
> >>> +}
> >>> +
> >>> +static const AMFTraceWriterVtbl tracer_vtbl =
> >>> +{
> >>> +.Write = AMFTraceWriter_Write,
> >>> +.Flush = AMFTraceWriter_Flush,
> >>
> >> Is this function really required to exist, given that it doesn't do
> >> anything?
> >>
> >>> +};
> >>> +
> >>> +static const AmfTraceWriter amf_trace_writer =
> >>> +{
> >>> +.vtbl = &tracer_vtbl,
> >>> +.avcl = &amflib_context,
> >>> +};
> >>
> >> This should probably be inside the AMFDeviceContextPrivate, so that it
> can
> >> point to the right context structure.
> >>
> >> This is the question.
> > AMF Library has global Trace settings, not per AMFContext object.
>
> So that global state is inside the AMF library?
>
> How does that interact with the fact that you reload it and reconfigure it
> every time it gets loaded - if one thread calls amf_device_create() while
> another one is inside the encoder and generating log output (say), what
> happens?
>

One of my first proposed patch had singleton amf_library, which was
refcounted. main goal of it was init library (set Trace options) when the
first reference is requested and clean when it was finally released (last
hwcontext_amf is destroyed).
I was told that global state is not good (global amf_lib object keeper) and
I removed it and now Tracer options are updated each time amf_device_create
is called.

Now there is no global state in avutils/hwcontext_amf : all globals here
are const static objects. The only global state in AMF Library itself
(configuring tracer is thread-safe in AMF).


> (I also note that it presumably means the current AMF encoder code will
> crash if you have two encoders with nested lifetimes - the second encoder
> will overwrite the global state, and once it finishes and gets freed the
> global trace output from the first encoder will have an invalid class
> pointer.)
>

Yes, I aware the issue since I started to support amfenc and this is the
first reason to move amf lib code to hwcontext_amf.



> > My intension was to create global AMF lib class and Tracer object which
> > refers to it as class parameter in av_log call.
> > It is required in scenario when multiple hwcontext_amf are created during
> > application lifecycle.
> > if this way is ok, should I add comments to code describes this? or is
> > there another way to have global object to handle this?
> Global state in libraries really isn't ok.  I think in response to that I
> would force it to be completely off by default, maybe switch it on if the
> user passes some special named option to amf_device_create()?
>

Probably this is the option, but current implementation should not cause
any issues except multiple work of tracer configuration.


> > AMFTraceWriterVtbl.Flush - I am not sure that it can be set as null
> > pointer, I could double check with AMD developers.
>
> It doesn't really matter; just the empty function didn't seem useful.
>
> >>>  #include 
> >>>  #include 
> >>
> >> Kindof unrelated, but is there any reason why both of these are in the
> >> header rather than in the per-codec files?
> >>
> >>
> > Component management code is the same for all encoder components.
> > The only encoder id defines is used for component creation here.
>
> Yep, missed that.  Sure.
>
> >>
> >> The log_to_dbg option is orphaned by this change.  Is it worth keeping?
> >> (If you want to keep it then maybe it

[FFmpeg-devel] [PATCH v4] avcodec: libdav1d AV1 decoder wrapper.

2018-10-29 Thread James Almer

From: "Ronald S. Bultje" 

Originally written by Ronald S. Bultje, with fixes, optimizations and
improvements by James Almer.

Signed-off-by: James Almer 
---
- s.n_frame_threads is now mapped to avctx->thread_count.
- colorspace, color_primaries and color_trc are now cast to avutil enums to
  silence -Wenum-conversion warnings on Clang.
- Constified the data pointer in libdav1d_data_free() callback due to recent
  changes upstream.

Dav1dData was updated upstream as well, so a { 0 } initialization should not
emit any kind of warning with old GCC/Clang anymore.

 configure  |   4 +
 libavcodec/Makefile|   1 +
 libavcodec/allcodecs.c |   1 +
 libavcodec/libdav1d.c  | 270 +
 4 files changed, 276 insertions(+)
 create mode 100644 libavcodec/libdav1d.c

diff --git a/configure b/configure
index 01c3a1011d..e56b925065 100755
--- a/configure
+++ b/configure
@@ -226,6 +226,7 @@ External library support:
   --enable-libcelt enable CELT decoding via libcelt [no]
   --enable-libcdio enable audio CD grabbing with libcdio [no]
   --enable-libcodec2   enable codec2 en/decoding using libcodec2 [no]
+  --enable-libdav1denable AV1 decoding via libdav1d [no]
   --enable-libdavs2enable AVS2 decoding via libdavs2 [no]
   --enable-libdc1394   enable IIDC-1394 grabbing using libdc1394
and libraw1394 [no]
@@ -1712,6 +1713,7 @@ EXTERNAL_LIBRARY_LIST="
 libcaca
 libcelt
 libcodec2
+libdav1d
 libdc1394
 libdrm
 libflite
@@ -3088,6 +3090,7 @@ libaom_av1_encoder_select="extract_extradata_bsf"
 libcelt_decoder_deps="libcelt"
 libcodec2_decoder_deps="libcodec2"
 libcodec2_encoder_deps="libcodec2"
+libdav1d_decoder_deps="libdav1d"
 libdavs2_decoder_deps="libdavs2"
 libfdk_aac_decoder_deps="libfdk_aac"
 libfdk_aac_encoder_deps="libfdk_aac"
@@ -6064,6 +6067,7 @@ enabled libcelt   && require libcelt celt/celt.h 
celt_decode -lcelt0 &&
die "ERROR: libcelt must be installed and 
version must be >= 0.11.0."; }
 enabled libcaca   && require_pkg_config libcaca caca caca.h 
caca_create_canvas
 enabled libcodec2 && require libcodec2 codec2/codec2.h codec2_create 
-lcodec2
+enabled libdav1d  && require_pkg_config libdav1d "dav1d >= 0.0.1" 
"dav1d/dav1d.h" dav1d_version
 enabled libdavs2  && require_pkg_config libdavs2 "davs2 >= 1.5.115" 
davs2.h davs2_decoder_open
 enabled libdc1394 && require_pkg_config libdc1394 libdc1394-2 
dc1394/dc1394.h dc1394_new
 enabled libdrm&& require_pkg_config libdrm libdrm xf86drm.h 
drmGetVersion
diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index 3e41497e34..8643da8f2b 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -956,6 +956,7 @@ OBJS-$(CONFIG_LIBAOM_AV1_ENCODER) += libaomenc.o
 OBJS-$(CONFIG_LIBCELT_DECODER)+= libcelt_dec.o
 OBJS-$(CONFIG_LIBCODEC2_DECODER)  += libcodec2.o codec2utils.o
 OBJS-$(CONFIG_LIBCODEC2_ENCODER)  += libcodec2.o codec2utils.o
+OBJS-$(CONFIG_LIBDAV1D_DECODER)   += libdav1d.o
 OBJS-$(CONFIG_LIBDAVS2_DECODER)   += libdavs2.o
 OBJS-$(CONFIG_LIBFDK_AAC_DECODER) += libfdk-aacdec.o
 OBJS-$(CONFIG_LIBFDK_AAC_ENCODER) += libfdk-aacenc.o
diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
index 1b8144a2b7..2c17db5a70 100644
--- a/libavcodec/allcodecs.c
+++ b/libavcodec/allcodecs.c
@@ -676,6 +676,7 @@ extern AVCodec ff_libaom_av1_encoder;
 extern AVCodec ff_libcelt_decoder;
 extern AVCodec ff_libcodec2_encoder;
 extern AVCodec ff_libcodec2_decoder;
+extern AVCodec ff_libdav1d_decoder;
 extern AVCodec ff_libdavs2_decoder;
 extern AVCodec ff_libfdk_aac_encoder;
 extern AVCodec ff_libfdk_aac_decoder;
diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c
new file mode 100644
index 00..873adfda40
--- /dev/null
+++ b/libavcodec/libdav1d.c
@@ -0,0 +1,270 @@
+/*
+ * Copyright (c) 2018 Ronald S. Bultje 
+ * Copyright (c) 2018 James Almer 
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include 
+
+#include "libavutil/avassert.h"
+#include "libavutil/fifo.h"
+#include "libavutil/opt.h"
+
+#include "avcodec.h"
+#include "decode.h"
+#incl

Re: [FFmpeg-devel] [PATCH] avcodec/vp9_parser: export profile and pixel format

2018-10-29 Thread Chris Cunningham

Thanks for pushing the profile part. Turns out I'm also interested in pixel
format, but I follow your comments about dropping the rest of patch. With
the code as-is, how busted are we when parsing a super frame. Does it
correctly grab the profile of the first frame?

Chrome actually has a fairly complete VP9 parser

but the CodecPrivate is no longer set in extradata for VP9 so we don't
(AFAIK) have easy access to the frame header. Would you be open to
surfacing codec private via extradata (reverting this change
)?

On Thu, Oct 25, 2018 at 4:50 PM James Almer  wrote:

> On 10/24/2018 3:12 PM, Chris Cunningham wrote:
> > On Wed, Oct 3, 2018 at 10:49 AM James Almer  wrote:
> >
> >> Signed-off-by: James Almer 
> >> ---
> >>  libavcodec/vp9_parser.c | 82 -
> >>  1 file changed, 80 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/libavcodec/vp9_parser.c b/libavcodec/vp9_parser.c
> >> index 9531f34a32..d4110f20bf 100644
> >> --- a/libavcodec/vp9_parser.c
> >> +++ b/libavcodec/vp9_parser.c
> >> @@ -23,36 +23,114 @@
> >>
> >>  #include "libavutil/intreadwrite.h"
> >>  #include "libavcodec/get_bits.h"
> >> +#include "libavcodec/internal.h"
> >>  #include "parser.h"
> >>
> >> +#define VP9_SYNCCODE 0x498342
> >> +
> >> +static int read_colorspace_details(AVCodecParserContext *ctx,
> >> AVCodecContext *avctx,
> >> +   GetBitContext *gb)
> >> +{
> >> +static const enum AVColorSpace colorspaces[8] = {
> >> +AVCOL_SPC_UNSPECIFIED, AVCOL_SPC_BT470BG, AVCOL_SPC_BT709,
> >> AVCOL_SPC_SMPTE170M,
> >> +AVCOL_SPC_SMPTE240M, AVCOL_SPC_BT2020_NCL, AVCOL_SPC_RESERVED,
> >> AVCOL_SPC_RGB,
> >> +};
> >> +int colorspace, bits = avctx->profile <= 1 ? 0 : 1 + get_bits1(gb);
> >> // 0:8, 1:10, 2:12
> >> +
> >> +colorspace = colorspaces[get_bits(gb, 3)];
> >> +if (colorspace == AVCOL_SPC_RGB) { // RGB = profile 1
> >> +static const enum AVPixelFormat pix_fmt_rgb[3] = {
> >> +AV_PIX_FMT_GBRP, AV_PIX_FMT_GBRP10, AV_PIX_FMT_GBRP12
> >> +};
> >> +if (avctx->profile & 1) {
> >> +if (get_bits1(gb)) // reserved bit
> >> +return AVERROR_INVALIDDATA;
> >> +} else
> >> +return AVERROR_INVALIDDATA;
> >> +ctx->format = pix_fmt_rgb[bits];
> >> +} else {
> >> +static const enum AVPixelFormat pix_fmt_for_ss[3][2 /* v */][2
> /*
> >> h */] = {
> >> +{ { AV_PIX_FMT_YUV444P,   AV_PIX_FMT_YUV422P },
> >> +  { AV_PIX_FMT_YUV440P,   AV_PIX_FMT_YUV420P } },
> >> +{ { AV_PIX_FMT_YUV444P10, AV_PIX_FMT_YUV422P10 },
> >> +  { AV_PIX_FMT_YUV440P10, AV_PIX_FMT_YUV420P10 } },
> >> +{ { AV_PIX_FMT_YUV444P12, AV_PIX_FMT_YUV422P12 },
> >> +  { AV_PIX_FMT_YUV440P12, AV_PIX_FMT_YUV420P12 } }
> >> +};
> >> +if (avctx->profile & 1) {
> >> +int ss_h, ss_v, format;
> >> +
> >> +ss_h = get_bits1(gb);
> >> +ss_v = get_bits1(gb);
> >> +format = pix_fmt_for_ss[bits][ss_v][ss_h];
> >> +if (format == AV_PIX_FMT_YUV420P)
> >> +// YUV 4:2:0 not supported in profiles 1 and 3
> >> +return AVERROR_INVALIDDATA;
> >> +else if (get_bits1(gb)) // color details reserved bit
> >> +return AVERROR_INVALIDDATA;
> >> +ctx->format = format;
> >> +} else {
> >> +ctx->format = pix_fmt_for_ss[bits][1][1];
> >> +}
> >> +}
> >> +
> >> +return 0;
> >> +}
> >> +
> >>  static int parse(AVCodecParserContext *ctx,
> >>   AVCodecContext *avctx,
> >>   const uint8_t **out_data, int *out_size,
> >>   const uint8_t *data, int size)
> >>  {
> >>  GetBitContext gb;
> >> -int res, profile, keyframe;
> >> +int res, profile, keyframe, invisible, errorres;
> >>
> >>  *out_data = data;
> >>  *out_size = size;
> >>
> >> -if ((res = init_get_bits8(&gb, data, size)) < 0)
> >> +if (!size || (res = init_get_bits8(&gb, data, size)) < 0)
> >>  return size; // parsers can't return errors
> >>  get_bits(&gb, 2); // frame marker
> >>  profile  = get_bits1(&gb);
> >>  profile |= get_bits1(&gb) << 1;
> >>  if (profile == 3) profile += get_bits1(&gb);
> >> +if (profile > 3)
> >> +return size;
> >>
> >> +avctx->profile = profile;
> >>  if (get_bits1(&gb)) {
> >>  keyframe = 0;
> >> +skip_bits(&gb, 3);
> >>  } else {
> >>  keyframe  = !get_bits1(&gb);
> >>  }
> >>
> >> +invisible = !get_bits1(&gb);
> >> +errorres  = get_bits1(&gb);
> >> +
> >>  if (!keyframe) {
> >> +int intraonly = invisible ? get_bits1(&gb) : 0;
> >>

Re: [FFmpeg-devel] [PATCH] avcodec/vp9_parser: export profile and pixel format

2018-10-29 Thread James Almer

On 10/29/2018 7:33 PM, Chris Cunningham wrote:
> Thanks for pushing the profile part. Turns out I'm also interested in
> pixel format, but I follow your comments about dropping the rest of
> patch. With the code as-is, how busted are we when parsing a super
> frame. Does it correctly grab the profile of the first frame?

It grabs the profile from the first frame in a super frame, which should
be safe since even if it's an invisible one it's not really going to be
different than the visible one, i think.

Also, when dealing with a super frame, to get other values like pixel
format and frame dimensions we'd have to make sure to parse the visible
frame, and if it's an inter frame also keep the reference frames around
to take said values from them.

> 
> Chrome actually has a fairly complete VP9 parser
> 
> but the CodecPrivate is no longer set in extradata for VP9 so we don't
> (AFAIK) have easy access to the frame header. Would you be open to
> surfacing codec private via extradata (reverting this change
> )?

I don't think that change should be reverted, at least not without
further changes. Otherwise, WebM specific extradata (the one defined in
https://www.webmproject.org/docs/container) could make its way to other
containers. We'd have to add checks in supported muxers to ignore it
instead.

Also, what muxer currently writes that to CodecPrivate? I've looked at
Youtube samples and none has it.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v4] avcodec: libdav1d AV1 decoder wrapper.

2018-10-29 Thread James Almer

> On Fri, Oct 26, 2018 at 4:00 PM James Almer  wrote:
> 
>> On 10/26/2018 7:50 PM, Dale Curtis wrote:
>>> One more piece of feedback, this is not obeying the
>>> AVCodecContext.get_buffer2 API.
>>
>> It's not using it on purpose, wrapping the buffers dav1d allocated
>> itself instead. Hence the lack of AV_CODEC_CAP_DR1 flag.
>>
> 
> Sorry for being unclear, my comment was a request. This is something
> Chromium would require to use this wrapper. If it's not planned, we'd forgo
> the wrapper and use dav1d directly. Otherwise we'd need to change our
> common ffmpeg based decoder enough for this one case that we'd be better
> off writing a pure dav1d based decoder. Thanks for your consideration in
> any case.
> 
> - dale
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 

I don't think the wrapper can use the get_buffer2 API if the idea is to
use it within Dav1dPicAllocator.alloc_picture_callback(). There's no way
to guarantee that the constrains required by said callback will be met
for example by a user provided implementation of get_buffer2.
Also, avctx dimensions need to be set by the time the
alloc_picture_callback() is called (as required by ff_get_buffer), and
said callback can't set them itself since we don't know at what point
during decoding it's going to be called (Things would probably go south
decoding a sample with frame changes in a frame multi-threading scenario).

If the idea is letting dav1d allocate its own buffers and then memcpy
them to get_buffer2() allocated buffers with av_image_copy(), like the
libaom wrapper is doing, then that can be done, but it's not something i
want committed since it will be slower than this zero copy implementation.
You could keep a local patch to implement it that way if you need to. It
should be trivial.

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 2/2] lavu/frame: Add error report if av_image_fill_pointers fail.

2018-10-29 Thread myp...@gmail.com

On Mon, Oct 29, 2018 at 11:20 PM Michael Niedermayer
 wrote:
>
> On Sun, Oct 28, 2018 at 11:05:47AM +0800, Jun Zhao wrote:
> > Add error handle if av_image_fill_pointers fail.
> >
> > Signed-off-by: Jun Zhao 
> > ---
> >  libavutil/frame.c |   10 ++
> >  1 files changed, 6 insertions(+), 4 deletions(-)
>
> should be ok
>
Will apply, Tks.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavc/mjpegdec: fix VA-API MJPEG decoding uninitialized huffman table

2018-10-29 Thread myp...@gmail.com

On Mon, Oct 29, 2018 at 6:39 PM Li, Zhong  wrote:
>
> > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> > Of Jun Zhao
> > Sent: Monday, October 29, 2018 6:26 PM
> > To: ffmpeg-devel@ffmpeg.org
> > Cc: Zhao, Jun ; Lin, Decai 
> > Subject: [FFmpeg-devel] [PATCH] lavc/mjpegdec: fix VA-API MJPEG decoding
> > uninitialized huffman table
> >
> > From: Jun Zhao 
> >
> > Now VA-API HWAccel MJPEG decoding uninitialized huffman table, it's will
> > lead to the decoding error like"Failed to sync surface 0x5: 23 (internal
> > decoding error)." in iHD open source driver.
> >
> > Signed-off-by: dlin2 
> > Signed-off-by: Jun Zhao 
> > ---
> >  libavcodec/mjpegdec.c |   19 +++
> >  1 files changed, 19 insertions(+), 0 deletions(-)
> >
> > diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c index
> > b0cb3ff..89effb6 100644
> > --- a/libavcodec/mjpegdec.c
> > +++ b/libavcodec/mjpegdec.c
> > @@ -75,6 +75,25 @@ static int build_vlc(VLC *vlc, const uint8_t *bits_table,
> > static int build_basic_mjpeg_vlc(MJpegDecodeContext *s)  {
> >  int ret;
> > +int i;
> > +
> > +/* initialize default huffman tables */
> > +for (i = 0; i < 16; i++)
> > +s->raw_huffman_lengths[0][0][i] =
> > avpriv_mjpeg_bits_dc_luminance[i + 1];
> > +for (i = 0; i < 12; i++)
> > +s->raw_huffman_values[0][0][i] = avpriv_mjpeg_val_dc[i];
> > +for (i = 0; i < 16; i++)
> > +s->raw_huffman_lengths[0][1][i] =
> > avpriv_mjpeg_bits_dc_chrominance[i + 1];
> > +for (i = 0; i < 12; i++)
> > +s->raw_huffman_values[0][1][i] = avpriv_mjpeg_val_dc[i];
> > +for (i = 0; i < 16; i++)
> > +s->raw_huffman_lengths[1][0][i] =
> > avpriv_mjpeg_bits_ac_luminance[i + 1];
> > +for (i = 0; i < 162; i++)
> > +s->raw_huffman_values[1][0][i] =
> > avpriv_mjpeg_val_ac_luminance[i];
> > +for (i = 0; i < 16; i++)
> > +s->raw_huffman_lengths[1][1][i] =
> > avpriv_mjpeg_bits_ac_chrominance[i + 1];
> > +for (i = 0; i < 162; i++)
> > +s->raw_huffman_values[1][1][i] =
> > + avpriv_mjpeg_val_ac_chrominance[i];
> >
> >  if ((ret = build_vlc(&s->vlcs[0][0], avpriv_mjpeg_bits_dc_luminance,
> >   avpriv_mjpeg_val_dc, 12, 0, 0)) < 0)
> > --
> > 1.7.1
>
> Should this be fixed in iHD driver instead of ffmpeg?
No, I don't think driver is a good place to initialize the huffman table.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] libavformat/http.c: add the protection about the http seek implement if a http resource is not streamed, it can seek to the end of this resouce.I add the detail information

2018-10-29 Thread Carl Eugen Hoyos

2018-10-29 10:42 GMT+01:00, shenqichao :
> Signed-off-by: shenqichao 

The first line of the commit message should not be too long
(<80 chars?), then there should be an empty line, then more
info if useful (like the ticket number).
Your commit message is only one line, but too long.

Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavc/mjpegdec: fix VA-API MJPEG decoding uninitialized huffman table

2018-10-29 Thread Carl Eugen Hoyos

2018-10-29 11:26 GMT+01:00, Jun Zhao :
> From: Jun Zhao 
>
> Now VA-API HWAccel MJPEG decoding uninitialized huffman
> table, it's will lead to the decoding error like"Failed to sync surface
> 0x5: 23 (internal decoding error)." in iHD open source driver.

This sounds as if the issue you want to fix is in the vaapi code
but your change affects only general mjpeg code.

Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] libavformat/http.c: add the protection about the http seek implement

2018-10-29 Thread shenqichao

If a http resource is not streamed, it can seek to the end of this resouce.

I add the detail information at https://trac.ffmpeg.org/ticket/6885

Fixes ticket #6885.

Signed-off-by: shenqichao 
---
 libavformat/http.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/http.c b/libavformat/http.c
index 3a35bc7eac..9401a5c63e 100644
--- a/libavformat/http.c
+++ b/libavformat/http.c
@@ -1669,7 +1669,7 @@ static int64_t http_seek_internal(URLContext *h, int64_t 
off, int whence, int fo
 int old_buf_size, ret;
 AVDictionary *options = NULL;
 
-if (whence == AVSEEK_SIZE)
+if (whence == AVSEEK_SIZE || (h->is_streamed == 0 &&  whence == SEEK_SET 
&& off == s->filesize))
 return s->filesize;
 else if (!force_reconnect &&
  ((whence == SEEK_CUR && off == 0) ||
-- 
2.19.0


___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavc/mjpegdec: fix VA-API MJPEG decoding uninitialized huffman table

2018-10-29 Thread myp...@gmail.com

On Tue, Oct 30, 2018 at 9:50 AM Carl Eugen Hoyos  wrote:
>
> 2018-10-29 11:26 GMT+01:00, Jun Zhao :
> > From: Jun Zhao 
> >
> > Now VA-API HWAccel MJPEG decoding uninitialized huffman
> > table, it's will lead to the decoding error like"Failed to sync surface
> > 0x5: 23 (internal decoding error)." in iHD open source driver.
>
> This sounds as if the issue you want to fix is in the vaapi code
> but your change affects only general mjpeg code.
>

Yes, as you know, HWaccel code is part of software decoder , so
we need to work on the general mjpeg code sometime, and maybe
I need to add a build macro in this case?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavc/mjpegdec: fix VA-API MJPEG decoding uninitialized huffman table

2018-10-29 Thread Li, Zhong

> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of myp...@gmail.com
> Sent: Tuesday, October 30, 2018 9:02 AM
> To: FFmpeg development discussions and patches
> 
> Cc: Zhao, Jun ; Lin, Decai 
> Subject: Re: [FFmpeg-devel] [PATCH] lavc/mjpegdec: fix VA-API MJPEG
> decoding uninitialized huffman table
> 
> On Mon, Oct 29, 2018 at 6:39 PM Li, Zhong  wrote:
> >
> > > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On
> > > Behalf Of Jun Zhao
> > > Sent: Monday, October 29, 2018 6:26 PM
> > > To: ffmpeg-devel@ffmpeg.org
> > > Cc: Zhao, Jun ; Lin, Decai 
> > > Subject: [FFmpeg-devel] [PATCH] lavc/mjpegdec: fix VA-API MJPEG
> > > decoding uninitialized huffman table
> > >
> > > From: Jun Zhao 
> > >
> > > Now VA-API HWAccel MJPEG decoding uninitialized huffman table, it's
> > > will lead to the decoding error like"Failed to sync surface 0x5: 23
> > > (internal decoding error)." in iHD open source driver.
> > >
> > > Signed-off-by: dlin2 
> > > Signed-off-by: Jun Zhao 
> > > ---
> > >  libavcodec/mjpegdec.c |   19 +++
> > >  1 files changed, 19 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c index
> > > b0cb3ff..89effb6 100644
> > > --- a/libavcodec/mjpegdec.c
> > > +++ b/libavcodec/mjpegdec.c
> > > @@ -75,6 +75,25 @@ static int build_vlc(VLC *vlc, const uint8_t
> > > *bits_table, static int build_basic_mjpeg_vlc(MJpegDecodeContext *s)
> {
> > >  int ret;
> > > +int i;
> > > +
> > > +/* initialize default huffman tables */
> > > +for (i = 0; i < 16; i++)
> > > +s->raw_huffman_lengths[0][0][i] =
> > > avpriv_mjpeg_bits_dc_luminance[i + 1];
> > > +for (i = 0; i < 12; i++)
> > > +s->raw_huffman_values[0][0][i] = avpriv_mjpeg_val_dc[i];
> > > +for (i = 0; i < 16; i++)
> > > +s->raw_huffman_lengths[0][1][i] =
> > > avpriv_mjpeg_bits_dc_chrominance[i + 1];
> > > +for (i = 0; i < 12; i++)
> > > +s->raw_huffman_values[0][1][i] = avpriv_mjpeg_val_dc[i];
> > > +for (i = 0; i < 16; i++)
> > > +s->raw_huffman_lengths[1][0][i] =
> > > avpriv_mjpeg_bits_ac_luminance[i + 1];
> > > +for (i = 0; i < 162; i++)
> > > +s->raw_huffman_values[1][0][i] =
> > > avpriv_mjpeg_val_ac_luminance[i];
> > > +for (i = 0; i < 16; i++)
> > > +s->raw_huffman_lengths[1][1][i] =
> > > avpriv_mjpeg_bits_ac_chrominance[i + 1];
> > > +for (i = 0; i < 162; i++)
> > > +s->raw_huffman_values[1][1][i] =
> > > + avpriv_mjpeg_val_ac_chrominance[i];
> > >
> > >  if ((ret = build_vlc(&s->vlcs[0][0],
> avpriv_mjpeg_bits_dc_luminance,
> > >   avpriv_mjpeg_val_dc, 12, 0, 0)) < 0)
> > > --
> > > 1.7.1
> >
> > Should this be fixed in iHD driver instead of ffmpeg?
> No, I don't think driver is a good place to initialize the huffman table.

For the default Huffman table, thus should be initialized by driver itself. 
For non-default case, thus should be passed from ffmpeg.
So, what is the case you want to fix? Both (if so, no need to mention iHD 
driver)? 

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavc/mjpegdec: fix VA-API MJPEG decoding uninitialized huffman table

2018-10-29 Thread myp...@gmail.com

On Tue, Oct 30, 2018 at 10:41 AM Li, Zhong  wrote:
>
> > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> > Of myp...@gmail.com
> > Sent: Tuesday, October 30, 2018 9:02 AM
> > To: FFmpeg development discussions and patches
> > 
> > Cc: Zhao, Jun ; Lin, Decai 
> > Subject: Re: [FFmpeg-devel] [PATCH] lavc/mjpegdec: fix VA-API MJPEG
> > decoding uninitialized huffman table
> >
> > On Mon, Oct 29, 2018 at 6:39 PM Li, Zhong  wrote:
> > >
> > > > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On
> > > > Behalf Of Jun Zhao
> > > > Sent: Monday, October 29, 2018 6:26 PM
> > > > To: ffmpeg-devel@ffmpeg.org
> > > > Cc: Zhao, Jun ; Lin, Decai 
> > > > Subject: [FFmpeg-devel] [PATCH] lavc/mjpegdec: fix VA-API MJPEG
> > > > decoding uninitialized huffman table
> > > >
> > > > From: Jun Zhao 
> > > >
> > > > Now VA-API HWAccel MJPEG decoding uninitialized huffman table, it's
> > > > will lead to the decoding error like"Failed to sync surface 0x5: 23
> > > > (internal decoding error)." in iHD open source driver.
> > > >
> > > > Signed-off-by: dlin2 
> > > > Signed-off-by: Jun Zhao 
> > > > ---
> > > >  libavcodec/mjpegdec.c |   19 +++
> > > >  1 files changed, 19 insertions(+), 0 deletions(-)
> > > >
> > > > diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c index
> > > > b0cb3ff..89effb6 100644
> > > > --- a/libavcodec/mjpegdec.c
> > > > +++ b/libavcodec/mjpegdec.c
> > > > @@ -75,6 +75,25 @@ static int build_vlc(VLC *vlc, const uint8_t
> > > > *bits_table, static int build_basic_mjpeg_vlc(MJpegDecodeContext *s)
> > {
> > > >  int ret;
> > > > +int i;
> > > > +
> > > > +/* initialize default huffman tables */
> > > > +for (i = 0; i < 16; i++)
> > > > +s->raw_huffman_lengths[0][0][i] =
> > > > avpriv_mjpeg_bits_dc_luminance[i + 1];
> > > > +for (i = 0; i < 12; i++)
> > > > +s->raw_huffman_values[0][0][i] = avpriv_mjpeg_val_dc[i];
> > > > +for (i = 0; i < 16; i++)
> > > > +s->raw_huffman_lengths[0][1][i] =
> > > > avpriv_mjpeg_bits_dc_chrominance[i + 1];
> > > > +for (i = 0; i < 12; i++)
> > > > +s->raw_huffman_values[0][1][i] = avpriv_mjpeg_val_dc[i];
> > > > +for (i = 0; i < 16; i++)
> > > > +s->raw_huffman_lengths[1][0][i] =
> > > > avpriv_mjpeg_bits_ac_luminance[i + 1];
> > > > +for (i = 0; i < 162; i++)
> > > > +s->raw_huffman_values[1][0][i] =
> > > > avpriv_mjpeg_val_ac_luminance[i];
> > > > +for (i = 0; i < 16; i++)
> > > > +s->raw_huffman_lengths[1][1][i] =
> > > > avpriv_mjpeg_bits_ac_chrominance[i + 1];
> > > > +for (i = 0; i < 162; i++)
> > > > +s->raw_huffman_values[1][1][i] =
> > > > + avpriv_mjpeg_val_ac_chrominance[i];
> > > >
> > > >  if ((ret = build_vlc(&s->vlcs[0][0],
> > avpriv_mjpeg_bits_dc_luminance,
> > > >   avpriv_mjpeg_val_dc, 12, 0, 0)) < 0)
> > > > --
> > > > 1.7.1
> > >
> > > Should this be fixed in iHD driver instead of ffmpeg?
> > No, I don't think driver is a good place to initialize the huffman table.
>
> For the default Huffman table, thus should be initialized by driver itself.
> For non-default case, thus should be passed from ffmpeg.
> So, what is the case you want to fix? Both (if so, no need to mention iHD 
> driver)?
Both, now HWAccel MJPEG always setting the huffman table and
can't distinguish the HWaccel JPEG whether default Huffman table.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 5/5] avformat:hlsenc.c: fix the output's duration smaller than input's in sub-range mode.

2018-10-29 Thread Steven Liu

Liu Steven  于2018年10月16日周二 上午11:17写道：
>
>
>
> > 在 2018年10月11日，下午4:56，Charles Liu  写道：
> >
> > In fmp4 & sub-range mode, the output's duration always smaller than 
> > expected, because the size of the last #EXT-X-BYTERANGE is too small.
> >
> > Signed-off-by: Charles Liu 
> > ---
> > libavformat/hlsenc.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> > index 8b3a9b78f4..f8f060d065 100644
> > --- a/libavformat/hlsenc.c
> > +++ b/libavformat/hlsenc.c
> > @@ -2380,6 +2380,7 @@ static int hls_write_trailer(struct AVFormatContext 
> > *s)
> >   if (ret < 0) {
> >   goto failed;
> >   }
> > +vs->size = range_length;
> >   ff_format_io_close(s, &vs->out);
> >   }
> >
> > @@ -2388,8 +2389,6 @@ failed:
> >   if (oc->pb) {
> >   if (hls->segment_type != SEGMENT_TYPE_FMP4) {
> >   vs->size = avio_tell(vs->avf->pb) - vs->start_pos;
> > -} else {
> > -vs->size = avio_tell(vs->avf->pb);
> >   }
> >   if (hls->segment_type != SEGMENT_TYPE_FMP4)
> >   ff_format_io_close(s, &oc->pb);
> > --
> > 2.19.1
> >
> > ___
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> Patchset will be pushed if there have no objections :)

Pushed

Thanks
>
>
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavc/mjpegdec: fix VA-API MJPEG decoding uninitialized huffman table

2018-10-29 Thread Eoff, Ullysses A

> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf Of 
> myp...@gmail.com
> Sent: Monday, October 29, 2018 8:10 PM
> To: FFmpeg development discussions and patches 
> Cc: Zhao, Jun ; Lin, Decai 
> Subject: Re: [FFmpeg-devel] [PATCH] lavc/mjpegdec: fix VA-API MJPEG decoding 
> uninitialized huffman table
> 
> On Tue, Oct 30, 2018 at 10:41 AM Li, Zhong  wrote:
> >
> > > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> > > Of myp...@gmail.com
> > > Sent: Tuesday, October 30, 2018 9:02 AM
> > > To: FFmpeg development discussions and patches
> > > 
> > > Cc: Zhao, Jun ; Lin, Decai 
> > > Subject: Re: [FFmpeg-devel] [PATCH] lavc/mjpegdec: fix VA-API MJPEG
> > > decoding uninitialized huffman table
> > >
> > > On Mon, Oct 29, 2018 at 6:39 PM Li, Zhong  wrote:
> > > >
> > > > > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On
> > > > > Behalf Of Jun Zhao
> > > > > Sent: Monday, October 29, 2018 6:26 PM
> > > > > To: ffmpeg-devel@ffmpeg.org
> > > > > Cc: Zhao, Jun ; Lin, Decai 
> > > > > Subject: [FFmpeg-devel] [PATCH] lavc/mjpegdec: fix VA-API MJPEG
> > > > > decoding uninitialized huffman table
> > > > >
> > > > > From: Jun Zhao 
> > > > >
> > > > > Now VA-API HWAccel MJPEG decoding uninitialized huffman table, it's
> > > > > will lead to the decoding error like"Failed to sync surface 0x5: 23
> > > > > (internal decoding error)." in iHD open source driver.
> > > > >
> > > > > Signed-off-by: dlin2 
> > > > > Signed-off-by: Jun Zhao 
> > > > > ---
> > > > >  libavcodec/mjpegdec.c |   19 +++
> > > > >  1 files changed, 19 insertions(+), 0 deletions(-)
> > > > >
> > > > > diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c index
> > > > > b0cb3ff..89effb6 100644
> > > > > --- a/libavcodec/mjpegdec.c
> > > > > +++ b/libavcodec/mjpegdec.c
> > > > > @@ -75,6 +75,25 @@ static int build_vlc(VLC *vlc, const uint8_t
> > > > > *bits_table, static int build_basic_mjpeg_vlc(MJpegDecodeContext *s)
> > > {
> > > > >  int ret;
> > > > > +int i;
> > > > > +
> > > > > +/* initialize default huffman tables */
> > > > > +for (i = 0; i < 16; i++)
> > > > > +s->raw_huffman_lengths[0][0][i] =
> > > > > avpriv_mjpeg_bits_dc_luminance[i + 1];
> > > > > +for (i = 0; i < 12; i++)
> > > > > +s->raw_huffman_values[0][0][i] = avpriv_mjpeg_val_dc[i];
> > > > > +for (i = 0; i < 16; i++)
> > > > > +s->raw_huffman_lengths[0][1][i] =
> > > > > avpriv_mjpeg_bits_dc_chrominance[i + 1];
> > > > > +for (i = 0; i < 12; i++)
> > > > > +s->raw_huffman_values[0][1][i] = avpriv_mjpeg_val_dc[i];
> > > > > +for (i = 0; i < 16; i++)
> > > > > +s->raw_huffman_lengths[1][0][i] =
> > > > > avpriv_mjpeg_bits_ac_luminance[i + 1];
> > > > > +for (i = 0; i < 162; i++)
> > > > > +s->raw_huffman_values[1][0][i] =
> > > > > avpriv_mjpeg_val_ac_luminance[i];
> > > > > +for (i = 0; i < 16; i++)
> > > > > +s->raw_huffman_lengths[1][1][i] =
> > > > > avpriv_mjpeg_bits_ac_chrominance[i + 1];
> > > > > +for (i = 0; i < 162; i++)
> > > > > +s->raw_huffman_values[1][1][i] =
> > > > > + avpriv_mjpeg_val_ac_chrominance[i];
> > > > >
> > > > >  if ((ret = build_vlc(&s->vlcs[0][0],
> > > avpriv_mjpeg_bits_dc_luminance,
> > > > >   avpriv_mjpeg_val_dc, 12, 0, 0)) < 0)
> > > > > --
> > > > > 1.7.1
> > > >
> > > > Should this be fixed in iHD driver instead of ffmpeg?
> > > No, I don't think driver is a good place to initialize the huffman table.
> >
> > For the default Huffman table, thus should be initialized by driver itself.
> > For non-default case, thus should be passed from ffmpeg.
> > So, what is the case you want to fix? Both (if so, no need to mention iHD 
> > driver)?
> Both, now HWAccel MJPEG always setting the huffman table and
> can't distinguish the HWaccel JPEG whether default Huffman table.

If your VA-API HWAccel MJPEG decoding case(s) work with i965 driver
and not iHD then I would assume iHD needs fixing.

> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] vaapi_encode: fix slices number check.

2018-10-29 Thread Jun Zhao

fix slice number check logic.

Signed-off-by: Jun Zhao 
---
 libavcodec/vaapi_encode.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
index 2fe8501..bf8f37b 100644
--- a/libavcodec/vaapi_encode.c
+++ b/libavcodec/vaapi_encode.c
@@ -1571,7 +1571,7 @@ static av_cold int 
vaapi_encode_init_slice_structure(AVCodecContext *avctx)
 return AVERROR(EINVAL);
 }
 
-if (ctx->nb_slices > avctx->slices) {
+if (ctx->nb_slices < avctx->slices) {
 av_log(avctx, AV_LOG_WARNING, "Slice count rounded up to "
"%d (from %d) due to driver constraints on slice "
"structure.\n", ctx->nb_slices, avctx->slices);
-- 
1.7.1

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavc/mjpegdec: fix VA-API MJPEG decoding uninitialized huffman table

2018-10-29 Thread myp...@gmail.com

On Tue, Oct 30, 2018 at 11:41 AM Eoff, Ullysses A
 wrote:
>
> > -Original Message-
> > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf Of 
> > myp...@gmail.com
> > Sent: Monday, October 29, 2018 8:10 PM
> > To: FFmpeg development discussions and patches 
> > Cc: Zhao, Jun ; Lin, Decai 
> > Subject: Re: [FFmpeg-devel] [PATCH] lavc/mjpegdec: fix VA-API MJPEG 
> > decoding uninitialized huffman table
> >
> > On Tue, Oct 30, 2018 at 10:41 AM Li, Zhong  wrote:
> > >
> > > > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> > > > Of myp...@gmail.com
> > > > Sent: Tuesday, October 30, 2018 9:02 AM
> > > > To: FFmpeg development discussions and patches
> > > > 
> > > > Cc: Zhao, Jun ; Lin, Decai 
> > > > Subject: Re: [FFmpeg-devel] [PATCH] lavc/mjpegdec: fix VA-API MJPEG
> > > > decoding uninitialized huffman table
> > > >
> > > > On Mon, Oct 29, 2018 at 6:39 PM Li, Zhong  wrote:
> > > > >
> > > > > > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On
> > > > > > Behalf Of Jun Zhao
> > > > > > Sent: Monday, October 29, 2018 6:26 PM
> > > > > > To: ffmpeg-devel@ffmpeg.org
> > > > > > Cc: Zhao, Jun ; Lin, Decai 
> > > > > > Subject: [FFmpeg-devel] [PATCH] lavc/mjpegdec: fix VA-API MJPEG
> > > > > > decoding uninitialized huffman table
> > > > > >
> > > > > > From: Jun Zhao 
> > > > > >
> > > > > > Now VA-API HWAccel MJPEG decoding uninitialized huffman table, it's
> > > > > > will lead to the decoding error like"Failed to sync surface 0x5: 23
> > > > > > (internal decoding error)." in iHD open source driver.
> > > > > >
> > > > > > Signed-off-by: dlin2 
> > > > > > Signed-off-by: Jun Zhao 
> > > > > > ---
> > > > > >  libavcodec/mjpegdec.c |   19 +++
> > > > > >  1 files changed, 19 insertions(+), 0 deletions(-)
> > > > > >
> > > > > > diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c index
> > > > > > b0cb3ff..89effb6 100644
> > > > > > --- a/libavcodec/mjpegdec.c
> > > > > > +++ b/libavcodec/mjpegdec.c
> > > > > > @@ -75,6 +75,25 @@ static int build_vlc(VLC *vlc, const uint8_t
> > > > > > *bits_table, static int build_basic_mjpeg_vlc(MJpegDecodeContext *s)
> > > > {
> > > > > >  int ret;
> > > > > > +int i;
> > > > > > +
> > > > > > +/* initialize default huffman tables */
> > > > > > +for (i = 0; i < 16; i++)
> > > > > > +s->raw_huffman_lengths[0][0][i] =
> > > > > > avpriv_mjpeg_bits_dc_luminance[i + 1];
> > > > > > +for (i = 0; i < 12; i++)
> > > > > > +s->raw_huffman_values[0][0][i] = avpriv_mjpeg_val_dc[i];
> > > > > > +for (i = 0; i < 16; i++)
> > > > > > +s->raw_huffman_lengths[0][1][i] =
> > > > > > avpriv_mjpeg_bits_dc_chrominance[i + 1];
> > > > > > +for (i = 0; i < 12; i++)
> > > > > > +s->raw_huffman_values[0][1][i] = avpriv_mjpeg_val_dc[i];
> > > > > > +for (i = 0; i < 16; i++)
> > > > > > +s->raw_huffman_lengths[1][0][i] =
> > > > > > avpriv_mjpeg_bits_ac_luminance[i + 1];
> > > > > > +for (i = 0; i < 162; i++)
> > > > > > +s->raw_huffman_values[1][0][i] =
> > > > > > avpriv_mjpeg_val_ac_luminance[i];
> > > > > > +for (i = 0; i < 16; i++)
> > > > > > +s->raw_huffman_lengths[1][1][i] =
> > > > > > avpriv_mjpeg_bits_ac_chrominance[i + 1];
> > > > > > +for (i = 0; i < 162; i++)
> > > > > > +s->raw_huffman_values[1][1][i] =
> > > > > > + avpriv_mjpeg_val_ac_chrominance[i];
> > > > > >
> > > > > >  if ((ret = build_vlc(&s->vlcs[0][0],
> > > > avpriv_mjpeg_bits_dc_luminance,
> > > > > >   avpriv_mjpeg_val_dc, 12, 0, 0)) < 0)
> > > > > > --
> > > > > > 1.7.1
> > > > >
> > > > > Should this be fixed in iHD driver instead of ffmpeg?
> > > > No, I don't think driver is a good place to initialize the huffman 
> > > > table.
> > >
> > > For the default Huffman table, thus should be initialized by driver 
> > > itself.
> > > For non-default case, thus should be passed from ffmpeg.
> > > So, what is the case you want to fix? Both (if so, no need to mention iHD 
> > > driver)?
> > Both, now HWAccel MJPEG always setting the huffman table and
> > can't distinguish the HWaccel JPEG whether default Huffman table.
>
> If your VA-API HWAccel MJPEG decoding case(s) work with i965 driver
> and not iHD then I would assume iHD needs fixing.
>
i965 must be have some issue if MJPEG decoding, when I try to dump the
data to CPU to check the frame

LIBVA_DRIVER_NAME=i965 ./ffmpeg -hwaccel vaapi -vaapi_device
/dev/dri/renderD128 -i
~/Videos/logitech_webcam_1280x720_30fps_2980frames.mjpeg -f null
/dev/null
ffmpeg version N-92305-g1ff4bd59df Copyright (c) 2000-2018 the FFmpeg developers
  built with gcc 7 (Ubuntu 7.3.0-27ubuntu1~18.04)
  configuration: --enable-libx264 --enable-libx265 --enable-gpl
--enable-libwebp --disable-optimizations --samples=../fate-suite
--enable-openssl --enable-nonfree --enable-libzmq --enable-libkvazaar
--enable-libvpx --enable-libvorbis --enable-libsrt
  libavutil  56. 21.100 / 5

Re: [FFmpeg-devel] [PATCH] avcodec/libvpxdec: fix setting auto threads

2018-10-29 Thread James Zern

On Sat, Oct 27, 2018 at 1:54 PM James Almer  wrote:
>
> On 10/27/2018 5:52 PM, James Zern wrote:
> > a thread count of 0 is treated the same as 1, use av_cpu_count() to get
> > the correct thread count when auto threads is requested.
> >
> > this matches the fix in libvpxenc:
> > 27df34bf1f avcodec/libvpxenc: fix setting amount of threads used for 
> > encoding
> >
> > Signed-off-by: James Zern 
> > ---
> >  libavcodec/libvpxdec.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/libavcodec/libvpxdec.c b/libavcodec/libvpxdec.c
> > index 04f27d3396..5193ac51c3 100644
> > --- a/libavcodec/libvpxdec.c
> > +++ b/libavcodec/libvpxdec.c
> > @@ -47,8 +47,7 @@ static av_cold int vpx_init(AVCodecContext *avctx,
> >  {
> >  VPxContext *ctx = avctx->priv_data;
> >  struct vpx_codec_dec_cfg deccfg = {
> > -/* token partitions+1 would be a decent choice */
> > -.threads = FFMIN(avctx->thread_count, 16)
> > +.threads = avctx->thread_count ? avctx->thread_count : 
> > av_cpu_count()
>
> If the limit of 16 threads is correct and unless libvpx clips it on its
> own, then you shouldn't remove the check.
>

If anything it was a limit put in at one stage of vp8 development, the
comment is certainly vp8 specific. vp8/9 both clip in their own ways
[1][2], but looking at it there may have been some changes to the loop
filter that aren't limiting this number. I'll restore it to be safe.
On the libvpxenc side I don't think there was ever a check in the
wrapper, but the library does put a hard cap on the requested threads
(64) and will fail otherwise.

[1] 
https://chromium.googlesource.com/webm/libvpx/+/v1.7.0/vp8/decoder/threading.c#596
[2] 
https://chromium.googlesource.com/webm/libvpx/+/v1.7.0/vp9/decoder/vp9_decodeframe.c#1542

> LGTM otherwise.
>
> >  };
> >
> >  av_log(avctx, AV_LOG_INFO, "%s\n", vpx_codec_version_str());
> >
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/libvpxdec: fix setting auto threads

2018-10-29 Thread James Zern

On Mon, Oct 29, 2018 at 10:58 PM James Zern  wrote:
>
> On Sat, Oct 27, 2018 at 1:54 PM James Almer  wrote:
> >
> > On 10/27/2018 5:52 PM, James Zern wrote:
> > > a thread count of 0 is treated the same as 1, use av_cpu_count() to get
> > > the correct thread count when auto threads is requested.
> > >
> > > this matches the fix in libvpxenc:
> > > 27df34bf1f avcodec/libvpxenc: fix setting amount of threads used for 
> > > encoding
> > >
> > > Signed-off-by: James Zern 
> > > ---
> > >  libavcodec/libvpxdec.c | 3 +--
> > >  1 file changed, 1 insertion(+), 2 deletions(-)
> > >

Applied with the limit of 16 restored. Thanks for the review.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/4] doc/filters: add document for opencl filters

2018-10-29 Thread Gyan

On Mon, Oct 29, 2018 at 10:50 AM Ruiling Song 
wrote:

> Signed-off-by: Danil Iashchenko 
> Signed-off-by: Ruiling Song 
> ---
> Seems like Danil is not working on this recently.
> So I re-submit this patch to address the comment over overlay_opencl.
>

Looks good for a push now. I'll make any changes later on.

Thanks,
Gyan
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 3/4] doc/filters: add tonemap_opencl document.

2018-10-29 Thread Gyan

On Mon, Oct 29, 2018 at 11:29 AM Ruiling Song 
wrote:

> Signed-off-by: Ruiling Song 
> ---
>  doc/filters.texi | 96
> 
>  1 file changed, 96 insertions(+)
>
> diff --git a/doc/filters.texi b/doc/filters.texi
> index 83df460..f884ba4 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -16387,6 +16387,7 @@ tmix=frames=3:weights="-1 2 -1":scale=1
>  @end example
>  @end itemize
>
> +@anchor{tonemap}
>  @section tonemap
>  Tone map colors from different dynamic ranges.
>
> @@ -18440,6 +18441,101 @@ Apply sobel operator with scale set to 2 and
> delta set to 10
>  @end example
>  @end itemize
>
> +@section tonemap_opencl
> +
> +Perform HDR(PQ/HLG) to SDR conversion with tone-mapping.
> +
> +It accepts the following parameters:
> +
> +@table @option
> +@item tonemap
> +Specify the tone-mapping operator to be used. Same as tonemap option in
> @ref{tonemap}.
> +
> +@item param
> +Tune the tone mapping algorithm. same as param option in @ref{tonemap}.
> +
> +@item desat
> +Apply desaturation for highlights that exceed this level of brightness.
> The
> +higher the parameter, the more color information will be preserved. This
> +setting helps prevent unnaturally blown-out colors for super-highlights,
> by
> +(smoothly) turning into white instead. This makes images feel more
> natural,
> +at the cost of reducing information about out-of-range colors.
> +
> +The default value is 0.5, and the algorithm here is a little different
> from
> +the cpu version tonemap currently. A setting of 0.0 disables this option.
> +
> +@item threshold
> +The tonemapping algorithm parameters is fine-tuned per each scene. And a
> threshold
> +is used to detect whether the scene has changed or not. If the distance
> beween
> +the current frame average brightness and the current running average
> exceeds
> +a threshold value, we would re-calculate scene average and peak
> brightness.
> +The default value is 0.2.
> +
> +@item format
> +Specify the output pixel format.
> +
> +Currently supported formats are:
> +@table @var
> +@item p010
> +@item nv12
> +@end table
> +
> +@item range, r
> +Set the output color range.
> +
> +Possible values are:
> +@table @var
> +@item tv/mpeg
> +@item pc/jpeg
> +@end table
> +
> +Default is same as input.
> +
> +@item primaries, p
> +Set the output color primaries.
> +
> +Possible values are:
> +@table @var
> +@item bt709
> +@item bt2020
> +@end table
> +
> +Default is same as input.
> +
> +@item transfer, t
> +Set the output transfer characteristics.
> +
> +Possible values are:
> +@table @var
> +@item bt709
> +@item bt2020
> +@end table
> +
> +Default is bt709.
> +
> +@item matrix, m
> +Set the output colorspace matrix.
> +
> +Possible value are:
> +@table @var
> +@item bt709
> +@item bt2020
> +@end table
> +
> +Default is same as input.
> +
> +@end table
> +
> +@subsection Example
> +
> +@itemize
> +@item
> +Convert HDR(PQ/HLG) video to bt2020-transfer-characteristic p010 format
> using linear operator.
> +@example
> +-i INPUT -vf
> "format=p010,hwupload,tonemap_opencl=t=bt2020:tonemap=linear:format=p010,hwdownload,format=p010"
> OUTPUT
> +@end example
> +@end itemize
> +
>  @section unsharp_opencl
>

LGTM.

Gyan
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

57 matches

Mail list logo