Re: [FFmpeg-devel] avcodec/proresenc_aw : add interlace encoding support

2019-02-10 Thread Kieran O Leary
Hi,

On Sat, Feb 9, 2019 at 6:10 PM Martin Vignali 
wrote:

> Hello,
>
> Patchs in attach add interlace encoding support to prores aw encoding
>

Thanks so much for adding this. I can really only judge by the metadata for
now, but this all looks good to me (ffmpeg encode and mediainfo check):

$ ./ffmpeg -f lavfi -i testsrc -field_order tt -flags ildct -c:v prores_aw
-t 10 tt_ildct_aw.mov && mediainfo tt_ildct_aw.mov |grep -i scan
ffmpeg version N-93103-gd20902fd23 Copyright (c) 2000-2019 the FFmpeg
developers
  built with gcc 7 (Ubuntu 7.3.0-27ubuntu1~18.04)
  configuration:
  libavutil  56. 26.100 / 56. 26.100
  libavcodec 58. 47.100 / 58. 47.100
  libavformat58. 26.101 / 58. 26.101
  libavdevice58.  6.101 / 58.  6.101
  libavfilter 7. 48.100 /  7. 48.100
  libswscale  5.  4.100 /  5.  4.100
  libswresample   3.  4.100 /  3.  4.100
Input #0, lavfi, from 'testsrc':
  Duration: N/A, start: 0.00, bitrate: N/A
Stream #0:0: Video: rawvideo (RGB[24] / 0x18424752), rgb24, 320x240
[SAR 1:1 DAR 4:3], 25 tbr, 25 tbn, 25 tbc
File 'tt_ildct_aw.mov' already exists. Overwrite ? [y/N] y
Stream mapping:
  Stream #0:0 -> #0:0 (rawvideo (native) -> prores (prores_aw))
Press [q] to stop, [?] for help
[prores_aw @ 0x55b56f330c80] encoding with ProRes  (ap4h) profile
[prores_aw @ 0x55b56f334180] encoding with ProRes  (ap4h) profile
[prores_aw @ 0x55b56f337580] encoding with ProRes  (ap4h) profile
[prores_aw @ 0x55b56f33aa00] encoding with ProRes  (ap4h) profile
[prores_aw @ 0x55b56f305ec0] encoding with ProRes  (ap4h) profile
Output #0, mov, to 'tt_ildct_aw.mov':
  Metadata:
encoder : Lavf58.26.101
Stream #0:0: Video: prores (prores_aw) () (ap4h / 0x68347061),
yuv444p10le(top first), 320x240 [SAR 1:1 DAR 4:3], q=2-31, 200 kb/s, 25
fps, 12800 tbn, 25 tbc
Metadata:
  encoder : Lavc58.47.100 prores_aw
frame=  250 fps=0.0 q=-0.0 Lsize=   10145kB time=00:00:09.96
bitrate=8344.2kbits/s speed=26.8x
video:10143kB audio:0kB subtitle:0kB other streams:0kB global headers:0kB
muxing overhead: 0.017619%
Scan type: Interlaced
Scan type, store method  : Separated fields (2 fields per
block)
Scan order   : Top Field First



>
> Use +ildct flag to switch between progressive and interlace encoding
>
> Use AVFrame flag to switch between tff and bff frame organization.
>

Is this what you mean by altering the -setparams filter?


> If AVFrame->interlaced value is not set to 1, but +ildct is set (interlace
> encoding of progressive frame), define field order to top (most common case
> for prores file)
>

It appears that prores_ks defaults to BFF?

$ ./ffmpeg -f lavfi -i testsrc  -flags ildct -c:v prores_ks -t 10
ildct_ks.mov && mediainfo ildct_ks.mov |grep -i scan
ffmpeg version N-93103-gd20902fd23 Copyright (c) 2000-2019 the FFmpeg
developers
  built with gcc 7 (Ubuntu 7.3.0-27ubuntu1~18.04)
  configuration:
  libavutil  56. 26.100 / 56. 26.100
  libavcodec 58. 47.100 / 58. 47.100
  libavformat58. 26.101 / 58. 26.101
  libavdevice58.  6.101 / 58.  6.101
  libavfilter 7. 48.100 /  7. 48.100
  libswscale  5.  4.100 /  5.  4.100
  libswresample   3.  4.100 /  3.  4.100
Input #0, lavfi, from 'testsrc':
  Duration: N/A, start: 0.00, bitrate: N/A
Stream #0:0: Video: rawvideo (RGB[24] / 0x18424752), rgb24, 320x240
[SAR 1:1 DAR 4:3], 25 tbr, 25 tbn, 25 tbc
File 'ildct_ks.mov' already exists. Overwrite ? [y/N] y
Stream mapping:
  Stream #0:0 -> #0:0 (rawvideo (native) -> prores (prores_ks))
Press [q] to stop, [?] for help
[prores_ks @ 0x557a246a5f80] Autoselected 4:4:4:4 profile because of the
used input colorspace. It can be overridden through -profile option.
[prores_ks @ 0x557a246b0e00] Autoselected 4:4:4:4 profile because of the
used input colorspace. It can be overridden through -profile option.
[prores_ks @ 0x557a246bbdc0] Autoselected 4:4:4:4 profile because of the
used input colorspace. It can be overridden through -profile option.
[prores_ks @ 0x557a246c6e40] Autoselected 4:4:4:4 profile because of the
used input colorspace. It can be overridden through -profile option.
[prores_ks @ 0x557a24677d80] Autoselected 4:4:4:4 profile because of the
used input colorspace. It can be overridden through -profile option.
Output #0, mov, to 'ildct_ks.mov':
  Metadata:
encoder : Lavf58.26.101
Stream #0:0: Video: prores (prores_ks) (ap4h / 0x68347061),
yuv444p10le, 320x240 [SAR 1:1 DAR 4:3], q=2-31, 200 kb/s, 25 fps, 12800
tbn, 25 tbc
Metadata:
  encoder : Lavc58.47.100 prores_ks
frame=   72 fps=0.0 q=-0.0 size=2816kB time=00:00:02.68
bitrate=8607.6kbits/frame=  144 fps=144 q=-0.0 size=5632kB
time=00:00:05.56 bitrate=8298.0kbits/frame=  216 fps=144 q=-0.0 size=
8704kB time=00:00:08.44 bitrate=8448.2kbits/frame=  250 fps=141 q=-0.0
Lsize=   10434kB time=00:00:09.96 bitrate=8582.2kbits/s speed=5.63x
video:10433kB audio:0kB subti

Re: [FFmpeg-devel] [PATCH] doc/filters: fix typos

2019-02-10 Thread Moritz Barsnick
On Sun, Feb 10, 2019 at 12:20:38 +0530, Gyan wrote:
> On 10-02-2019 11:37 AM, Reto Kromer wrote:
> > Should fix a few nits in man. Best regards, Reto
> 
> Removed trailing whitespace from patch and pushed as 
> d20902fd2399ae04cbd5c02e83fbd90c68592555 with a couple of corrections added.

Hmm, these looked so familiar too me. I had caught most of these and
some more here:

https://patchwork.ffmpeg.org/patch/11935/

Looks like I need to rebase and resubmit. ;-)
(I'll follow up with a V2 to that e-mail.)

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


[FFmpeg-devel] [PATCH] doc: fix various typos

2019-02-10 Thread Moritz Barsnick
Found with the help of codespell-1.14.0.

Signed-off-by: Moritz Barsnick 
---
 doc/bitstream_filters.texi |  2 +-
 doc/codecs.texi|  2 +-
 doc/filters.texi   | 16 
 doc/formats.texi   |  2 +-
 doc/general.texi   |  8 
 doc/muxers.texi|  2 +-
 6 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/doc/bitstream_filters.texi b/doc/bitstream_filters.texi
index b779265f58..076b910e40 100644
--- a/doc/bitstream_filters.texi
+++ b/doc/bitstream_filters.texi
@@ -561,7 +561,7 @@ P3 D65
 @end table
 
 @item transfer_characteristics
-Set the color transfert.
+Set the color transfer.
 Available values are:
 
 @table @samp
diff --git a/doc/codecs.texi b/doc/codecs.texi
index 02d5a1b222..c1c6996dee 100644
--- a/doc/codecs.texi
+++ b/doc/codecs.texi
@@ -1230,7 +1230,7 @@ instead of alpha. Default is 0.
 @item dump_separator @var{string} (@emph{input})
 Separator used to separate the fields printed on the command line about the
 Stream parameters.
-For example to separate the fields with newlines and indention:
+For example to separate the fields with newlines and indentation:
 @example
 ffprobe -dump_separator "
   "  -i ~/videos/matrixbench_mpeg2.mpg
diff --git a/doc/filters.texi b/doc/filters.texi
index ac6f6f0082..0ef6f56d5d 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -606,7 +606,7 @@ The lower value, the more samples will be detected as 
impulsive noise.
 Set burst fusion, in percentage of window size. Allowed range is @code{0} to
 @code{10}. Default value is @code{2}.
 If any two samples detected as noise are spaced less than this value then any
-sample in between those two samples will be also detected as noise.
+sample between those two samples will be also detected as noise.
 
 @item m
 Set overlap method.
@@ -1399,7 +1399,7 @@ single-precision floating-point
 @end table
 
 @item response
-Show IR frequency reponse, magnitude and phase in additional video stream.
+Show IR frequency response, magnitude and phase in additional video stream.
 By default it is disabled.
 
 @item channel
@@ -1425,7 +1425,7 @@ used for all remaining channels.
 
 @itemize
 @item
-Apply 2 pole elliptic notch at arround 5000Hz for 48000 Hz sample rate:
+Apply 2 pole elliptic notch at around 5000Hz for 48000 Hz sample rate:
 @example
 aiir=k=1:z=7.957584807809675810E-1 -2.575128568908332300 3.674839853930788710 
-2.57512875289799137 7.957586296317130880E-1:p=1 -2.86950072432325953 
3.63022088054647218 -2.28075678147272232 6.361362326477423500E-1:f=tf:r=d
 @end example
@@ -5648,7 +5648,7 @@ For example radius of 3 will instruct filter to calculate 
average of 7 frames.
 Set factor to amplify difference. Default is 2. Allowed range is from 0 to 
65535.
 
 @item threshold
-Set threshold for difference amplification. Any differrence greater or equal to
+Set threshold for difference amplification. Any difference greater or equal to
 this value will not alter source pixel. Default is 10.
 Allowed range is from 0 to 65535.
 
@@ -6033,7 +6033,7 @@ The filter accepts the following options.
 @item sigma
 Set denoising strength. Default value is 1.
 Allowed range is from 0 to 999.9.
-The denoising algorith is very sensitive to sigma, so adjust it
+The denoising algorithm is very sensitive to sigma, so adjust it
 according to the source.
 
 @item block
@@ -10265,7 +10265,7 @@ The filter accepts the following options:
 Set horizontal sigma, standard deviation of Gaussian blur. Default is 
@code{0.5}.
 
 @item steps
-Set number of steps for Gaussian approximation. Defauls is @code{1}.
+Set number of steps for Gaussian approximation. Default is @code{1}.
 
 @item planes
 Set which planes to filter. By default all planes are filtered.
@@ -15336,7 +15336,7 @@ Keep the same color primaries property (default).
 @end table
 
 @item color_trc
-Set the color transfert.
+Set the color transfer.
 Available values are:
 
 @table @samp
@@ -20136,7 +20136,7 @@ Default is @code{log}.
 
 @item acount
 Set how much frames to accumulate in histogram.
-Defauls is 1. Setting this to -1 accumulates all frames.
+Default is 1. Setting this to -1 accumulates all frames.
 
 @item rheight
 Set histogram ratio of window height.
diff --git a/doc/formats.texi b/doc/formats.texi
index 4f334e03c7..3cee7767d8 100644
--- a/doc/formats.texi
+++ b/doc/formats.texi
@@ -211,7 +211,7 @@ is @code{0} (meaning that no offset is applied).
 @item dump_separator @var{string} (@emph{input})
 Separator used to separate the fields printed on the command line about the
 Stream parameters.
-For example to separate the fields with newlines and indention:
+For example to separate the fields with newlines and indentation:
 @example
 ffprobe -dump_separator "
   "  -i ~/videos/matrixbench_mpeg2.mpg
diff --git a/doc/general.texi b/doc/general.texi
index 2bc33d180d..fe94c40386 100644
--- a/doc/general.texi
+++ b/doc/general.texi
@@ -1135,10 +1135,10 @@ following ima

Re: [FFmpeg-devel] avcodec/proresenc_aw : add interlace encoding support

2019-02-10 Thread Martin Vignali
Hello,

>
> > Use +ildct flag to switch between progressive and interlace encoding
> >
> > Use AVFrame flag to switch between tff and bff frame organization.
> >
>
> Is this what you mean by altering the -setparams filter?
>
> In order to choose between top field first and bottom field first, i use
the AVFrame property (interlaced, and top_field_first)
(these property can be set using setparams filter using field_mode (so
setparams make a more predictable result of field order (and color property
(if set) at the same time))


> > If AVFrame->interlaced value is not set to 1, but +ildct is set
> (interlace
> > encoding of progressive frame), define field order to top (most common
> case
> > for prores file)
> >
>
> It appears that prores_ks defaults to BFF?
>

Following source code of prores_ks, i think prores_ks, doesn't really make
a choice for interlace encoding of not interlace content
it test for top field first property, if not set define to bottom (but
doesn't check the interlaced flag before)
Make more sense to me to convert progressive content to top field first.

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


Re: [FFmpeg-devel] avcodec/proresenc_aw : add interlace encoding support

2019-02-10 Thread Kieran O Leary
On Sun, 10 Feb 2019, 11:57 Martin Vignali  Hello,
>
> >
> > > Use +ildct flag to switch between progressive and interlace encoding
> > >
> > > Use AVFrame flag to switch between tff and bff frame organization.
> > >
> >
> > Is this what you mean by altering the -setparams filter?
> >
> > In order to choose between top field first and bottom field first, i use
> the AVFrame property (interlaced, and top_field_first)
> (these property can be set using setparams filter using field_mode (so
> setparams make a more predictable result of field order (and color property
> (if set) at the same time))
>
>
> > > If AVFrame->interlaced value is not set to 1, but +ildct is set
> > (interlace
> > > encoding of progressive frame), define field order to top (most common
> > case
> > > for prores file)
> > >
> >
> > It appears that prores_ks defaults to BFF?
> >
>
> Following source code of prores_ks, i think prores_ks, doesn't really make
> a choice for interlace encoding of not interlace content
> it test for top field first property, if not set define to bottom (but
> doesn't check the interlaced flag before)
> Make more sense to me to convert progressive content to top field first.
>

I've no preference,I was just curious why there was a difference. Thanks
again for adding this. It will be very useful for those of us still dealing
with PAL/NTSC.

Best,

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


Re: [FFmpeg-devel] lavc/libvpxenc: Deprecate lossless option

2019-02-10 Thread Kieran O Leary
On Sat, 9 Feb 2019, 06:49 Gyan 
>
> On 09-02-2019 02:26 AM, Carl Eugen Hoyos wrote:
> > 2019-02-08 6:08 GMT+01:00, Gyan :
> >>
> >> On 08-02-2019 03:31 AM, Carl Eugen Hoyos wrote:
> >>> .
> >>> No strong opinion here, I hadn't realized that -crf 0 only works with
> >>> newer versions.
> >>>
> >>> Can you explain why crf alone has no effect and needs -b:v 0?
> >>> Isn't this a bug both with libvpx and libaom?
> >>>
> >> With crf present but VBV params absent, VPX operates using a constrained
> >> Q RC mode , where the target bitrate acts as a ceiling. Since acvodec
> >> has a non-zero default -b of 200 kbps, this produces undesirable
> >> effects. If set to 0, VPX switches to constant quality.
> > Yes.
> > This looks like a bug to me.
> If there's a bug, it's in libav, in that, we can't signal when a value
> is assigned by the user or via user-initiated logic, versus being
> assigned as a default fallback. So it's a design conflict, and the
> workaround has been long documented.
>
> Maybe just before the next major bump, a new field should be introduced
> in AVOptions which registers if the field was populated at the behest of
> the user.
> >> I do see this block though,
> >>
> >>   if (avctx->codec_id == AV_CODEC_ID_VP9 && ctx->lossless == 1) {
> >>   enccfg.rc_min_quantizer =
> >>   enccfg.rc_max_quantizer = 0;
> >>   } else {
> >>   if (avctx->qmin >= 0)
> >>   enccfg.rc_min_quantizer = avctx->qmin;
> >>   if (avctx->qmax >= 0)
> >>   enccfg.rc_max_quantizer = avctx->qmax;
> >>   }
> >>
> >>
> >> Looks like the quantizer range is disabled only by using the deprecated
> >> option, or has this changed?
> > Is your question "Isn't 'lossless 1' necessary for lossless encoding"?
> Yes. A quick glance at vpx HEAD indicates it is, although -qmin & -qmax
> 0 should also work.
>
> >> Also, with libvpx v1.7.0-1758, I get different results for -crf 0 -b:v
> >> 0  vs only -lossless 1, with the latter producing a slightly larger
> >> file, and its result showing a slightly larger SSIM score. Although
> >> neither produces a SSIM of 1, like libx264. Not truly lossless?
> > Please provide your input sample.
> I can reproduce the difference in result with
> fate-suite/h264-conformance/src19td.IBP.264
>
> although the `-lossless 1` encoding does return as lossless in SSIM.
>

What ssim command did you use, and why use this over a hash muxer like
framehash? I'm always on the lookout for losslessness verification methods.

Best,

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


[FFmpeg-devel] doc/snow: fix typos

2019-02-10 Thread Reto Kromer

Best regards, Reto

0001-doc-snow-fix-typos.patch
Description: Binary data
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 1/2] libavcodec/zmbvenc: block scoring improvements/bug fixes

2019-02-10 Thread Tomas Härdin
lör 2019-02-09 klockan 13:10 + skrev Matthew Fearnley:
> - Improve block choices by counting 0-bytes in the entropy score
> - Make histogram use uint16_t type, to allow byte counts from 16*16
> (current block size) up to 255*255 (maximum allowed 8bpp block size)
> - Make sure score table is big enough for a full block's worth of bytes
> - Calculate *xored without using code in inner loop
> ---
>  libavcodec/zmbvenc.c | 22 --
>  1 file changed, 16 insertions(+), 6 deletions(-)

Passes FATE, looks good to me

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


Re: [FFmpeg-devel] [PATCH] doc/filters: fix typos

2019-02-10 Thread Gyan



On 10-02-2019 04:41 PM, Moritz Barsnick wrote:

On Sun, Feb 10, 2019 at 12:20:38 +0530, Gyan wrote:

On 10-02-2019 11:37 AM, Reto Kromer wrote:

Should fix a few nits in man. Best regards, Reto

Removed trailing whitespace from patch and pushed as
d20902fd2399ae04cbd5c02e83fbd90c68592555 with a couple of corrections added.

Hmm, these looked so familiar too me. I had caught most of these and
some more here:

https://patchwork.ffmpeg.org/patch/11935/

Looks like I need to rebase and resubmit. ;-)
(I'll follow up with a V2 to that e-mail.)


Pushed as 885a80d189698633e7c94bb55fdb1dac12871483

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


Re: [FFmpeg-devel] doc/snow: fix typos

2019-02-10 Thread Carl Eugen Hoyos
2019-02-10 13:32 GMT+01:00, Reto Kromer :
> Best regards, Reto

Patch applied.

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


Re: [FFmpeg-devel] [PATCH 2/2] libavcodec/zmbvenc: motion estimation improvements/bug fixes:

2019-02-10 Thread Tomas Härdin
lör 2019-02-09 klockan 13:10 + skrev Matthew Fearnley:
> - Clamp ME range to -64..63 (prevents corruption when me_range is too high)
> - Allow MV's up to *and including* the positive range limit
> - Allow out-of-edge ME by padding the prev buffer with a border of 0's
> - Try previous MV before checking the rest (improves speed in some cases)
> - More robust logic in code - ensure *mx,*my,*xored are updated together
> ---
>  libavcodec/zmbvenc.c | 64 +++-
>  1 file changed, 46 insertions(+), 18 deletions(-)

Passes FATE

The only maybe suspicious thing is this part:

> -c->pstride = FFALIGN(avctx->width, 16);
> -if (!(c->prev = av_malloc(c->pstride * avctx->height))) {
> +
> +/* Allocate prev buffer - leave border around the outside for out of 
> edge ME */
> +c->pstride = FFALIGN(avctx->width + c->lrange, 16);

Shouldn't this be with + lrange + urange? I guess it works out fine due
to wraparound and lrange >= urange, but it makes me feel slightly
uneasy

> +prev_offset = FFALIGN(c->lrange + (c->pstride * c->lrange), 16);
> +prev_size = prev_offset + (c->pstride * (avctx->height + c->urange));

The way I'd do this is compute the size first, then the offset. But I
guess this works out the same way. Maybe someone else wants to chime
in?

/Tomas

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


[FFmpeg-devel] [PATCH] doc/faq: update macOS and URLs

2019-02-10 Thread Reto Kromer

Best regards, Reto

0001-doc-faq-update-macOS-and-URLs.patch
Description: Binary data
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH]lavf/img2dec: Split img2 and img2pipe options

2019-02-10 Thread Carl Eugen Hoyos
2019-02-10 1:55 GMT+01:00, Jan Ekström :
> On Sat, Feb 9, 2019 at 2:58 PM Carl Eugen Hoyos  wrote:
>>
>> Hi!
>>
>> Currently if the "loop" option gets specified for -f image2pipe, there is
>> no indication for the user that the option will not work, patch attached.
>>
>> Please comment, Carl Eugen
>
> Seems like a good idea.
>
> The reason for the pipe version's AVOption structure to be outside of
> the if is the IMAGEAUTO_DEMUXER macro, so I guess unless we like
> really long #ifs then this is as good as it gets.

An alternative is to move the options into the IMAGEAUTO_DEMUXER
macro but I did not see an advantage.

> Quickly checked and it seems like generally the separation of options
> seems correct... but I'm not sure about "ts_from_file" . The filename
> variable will probably be full of \0s at the point where stat() is
> called, as it's only set to something else under "!s->is_pipe" branch
> in the beginning of the function if I'm seeing correctly - thus making
> this option effectively unusable under the _pipe demuxers?

Definitely, I left ts_from_file where it was and pushed.

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


Re: [FFmpeg-devel] [PATCH]lavf/img2dec: Split img2 and img2pipe options

2019-02-10 Thread Michael Niedermayer
On Sat, Feb 09, 2019 at 01:58:06PM +0100, Carl Eugen Hoyos wrote:
> Hi!
> 
> Currently if the "loop" option gets specified for -f image2pipe, there is
> no indication for the user that the option will not work, patch attached.
> 
> Please comment, Carl Eugen

>  img2dec.c |   33 -
>  1 file changed, 20 insertions(+), 13 deletions(-)
> eb41db12f922fc0780a808df6d8f832620ef9d65  
> 0001-lavf-img2dec-Split-img2-and-img2pipe-options.patch
> From 21b11da9af5166e4bda7398a471a1e1a49352ac3 Mon Sep 17 00:00:00 2001
> From: Carl Eugen Hoyos 
> Date: Sat, 9 Feb 2019 13:49:51 +0100
> Subject: [PATCH] lavf/img2dec: Split img2 and img2pipe options.
> 
> ---
>  libavformat/img2dec.c |   33 -
>  1 file changed, 20 insertions(+), 13 deletions(-)

this breaks -loop

./ffmpeg -loop 1 -i ~/fatesamples/png1/lena-rgb24.png -t 1 test.avi

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Observe your enemies, for they first find out your faults. -- Antisthenes


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


Re: [FFmpeg-devel] [PATCH]lavf/img2dec: Split img2 and img2pipe options

2019-02-10 Thread Carl Eugen Hoyos
2019-02-10 18:44 GMT+01:00, Michael Niedermayer :
> On Sat, Feb 09, 2019 at 01:58:06PM +0100, Carl Eugen Hoyos wrote:
>> Hi!
>>
>> Currently if the "loop" option gets specified for -f image2pipe, there is
>> no indication for the user that the option will not work, patch attached.
>>
>> Please comment, Carl Eugen
>
>>  img2dec.c |   33 -
>>  1 file changed, 20 insertions(+), 13 deletions(-)
>> eb41db12f922fc0780a808df6d8f832620ef9d65
>> 0001-lavf-img2dec-Split-img2-and-img2pipe-options.patch
>> From 21b11da9af5166e4bda7398a471a1e1a49352ac3 Mon Sep 17 00:00:00 2001
>> From: Carl Eugen Hoyos 
>> Date: Sat, 9 Feb 2019 13:49:51 +0100
>> Subject: [PATCH] lavf/img2dec: Split img2 and img2pipe options.
>>
>> ---
>>  libavformat/img2dec.c |   33 -
>>  1 file changed, 20 insertions(+), 13 deletions(-)
>
> this breaks -loop
>
> ./ffmpeg -loop 1 -i ~/fatesamples/png1/lena-rgb24.png -t 1 test.avi

I moved the option that is obviously needed for the auto-detecting demuxers.

The original issue was that the image2pipe demuxer happily accepts
"loop" but ignores it - confusing users, I wonder if the only solution is
to split the options between image2pipe and the auto-detecting
demuxers or if there is another solution (or if both patches should
be reverted).

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


Re: [FFmpeg-devel] [PATCH]lavf/img2dec: Split img2 and img2pipe options

2019-02-10 Thread Gyan



On 10-02-2019 11:31 PM, Carl Eugen Hoyos wrote:

2019-02-10 18:44 GMT+01:00, Michael Niedermayer :

On Sat, Feb 09, 2019 at 01:58:06PM +0100, Carl Eugen Hoyos wrote:

Hi!

Currently if the "loop" option gets specified for -f image2pipe, there is
no indication for the user that the option will not work, patch attached.

Please comment, Carl Eugen
  img2dec.c |   33 -
  1 file changed, 20 insertions(+), 13 deletions(-)
eb41db12f922fc0780a808df6d8f832620ef9d65
0001-lavf-img2dec-Split-img2-and-img2pipe-options.patch
 From 21b11da9af5166e4bda7398a471a1e1a49352ac3 Mon Sep 17 00:00:00 2001
From: Carl Eugen Hoyos 
Date: Sat, 9 Feb 2019 13:49:51 +0100
Subject: [PATCH] lavf/img2dec: Split img2 and img2pipe options.

---
  libavformat/img2dec.c |   33 -
  1 file changed, 20 insertions(+), 13 deletions(-)

this breaks -loop

./ffmpeg -loop 1 -i ~/fatesamples/png1/lena-rgb24.png -t 1 test.avi

I moved the option that is obviously needed for the auto-detecting demuxers.

The original issue was that the image2pipe demuxer happily accepts
"loop" but ignores it - confusing users,
I suggest to keep ignoring it and print a warning for is_pipe && loop in 
read_header.


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


Re: [FFmpeg-devel] [PATCH v2 06/11] vaapi_encode_vp9: Enable support for more RC modes

2019-02-10 Thread Mark Thompson
On 05/02/2019 13:25, Carl Eugen Hoyos wrote:
> 2019-01-28 0:47 GMT+01:00, Mark Thompson :
>> ---
>>  libavcodec/vaapi_encode_vp9.c | 41 +--
>>  1 file changed, 25 insertions(+), 16 deletions(-)
>>
>> diff --git a/libavcodec/vaapi_encode_vp9.c b/libavcodec/vaapi_encode_vp9.c
>> index 97142dcc49..f89fd0d07a 100644
>> --- a/libavcodec/vaapi_encode_vp9.c
>> +++ b/libavcodec/vaapi_encode_vp9.c
>> @@ -178,23 +178,29 @@ static int
>> vaapi_encode_vp9_init_picture_params(AVCodecContext *avctx,
>>
>>  static av_cold int vaapi_encode_vp9_configure(AVCodecContext *avctx)
>>  {
>> +VAAPIEncodeContext *ctx = avctx->priv_data;
>>  VAAPIEncodeVP9Context *priv = avctx->priv_data;
>>
>> -priv->q_idx_p = av_clip(avctx->global_quality, 0, VP9_MAX_QUANT);
>> -if (avctx->i_quant_factor > 0.0)
>> -priv->q_idx_idr = av_clip((avctx->global_quality *
>> -   avctx->i_quant_factor +
>> -   avctx->i_quant_offset) + 0.5,
>> -  0, VP9_MAX_QUANT);
>> -else
>> -priv->q_idx_idr = priv->q_idx_p;
>> -if (avctx->b_quant_factor > 0.0)
>> -priv->q_idx_b = av_clip((avctx->global_quality *
>> - avctx->b_quant_factor +
>> - avctx->b_quant_offset) + 0.5,
>> -0, VP9_MAX_QUANT);
>> -else
>> -priv->q_idx_b = priv->q_idx_p;
>> +if (ctx->rc_mode->quality) {
>> +priv->q_idx_p = av_clip(ctx->rc_quality, 0, VP9_MAX_QUANT);
>> +if (avctx->i_quant_factor > 0.0)
>> +priv->q_idx_idr =
>> +av_clip((avctx->i_quant_factor * priv->q_idx_p  +
>> + avctx->i_quant_offset) + 0.5,
>> +0, VP9_MAX_QUANT);
>> +else
>> +priv->q_idx_idr = priv->q_idx_p;
>> +if (avctx->b_quant_factor > 0.0)
>> +priv->q_idx_b =
>> +av_clip((avctx->b_quant_factor * priv->q_idx_p  +
>> + avctx->b_quant_offset) + 0.5,
>> +0, VP9_MAX_QUANT);
>> +else
>> +priv->q_idx_b = priv->q_idx_p;
> 
> I will not work on this code, so I shouldn't care but this
> is an exceptional example for an unreadable patch.

Yeah, that's probably fair.  I've split this into functional and cosmetic parts 
in a new version.

Thanks,

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


Re: [FFmpeg-devel] [PATCH v2 08/11] vaapi_encode_mjpeg: Warn if input is not full range

2019-02-10 Thread Mark Thompson
On 05/02/2019 13:27, Carl Eugen Hoyos wrote:
> 2019-01-28 0:47 GMT+01:00, Mark Thompson :
> 
>> +if (avctx->color_range == AVCOL_RANGE_MPEG) {
>> +av_log(avctx, AV_LOG_WARNING, "Input video does not appear "
>> +   "to use full-range: output colours may be incorrect.\n");
> 
> The wording seems not ideal to me:
> Are you not sure if the encoder will produce correct output for
> mpeg-range jpeg?
> That's how it sounds to me...

The problem is that, unlike H.264 and other codecs, JPEG has no way to signal 
that the data are actually limited-range.  The encode will preserve the values, 
but those values will not have the right interpretation at a remote end which 
only looks at the JPEG data.  On the other hand, if signalled by some 
out-of-band method (perhaps the container, or you control both ends) everything 
is perfectly usable.

I agree the message isn't particularly clear; do you have any thoughts about 
how it should be phrased?

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


Re: [FFmpeg-devel] lavc/libvpxenc: Deprecate lossless option

2019-02-10 Thread Gyan



On 10-02-2019 05:36 PM, Kieran O Leary wrote:


What ssim command did you use, and why use this over a hash muxer like
framehash? I'm always on the lookout for losslessness verification methods.


The barebones command:

    ffmpeg -i main -i ref -lavfi ssim -f null -

framehash will be sensitive to data layout, so that comparing the same 
payload in planar vs packed format will yield different values. As the 
SSIM filter only works with planar formats, avfilter will take care of 
conversion, if needed, of both inputs.


Other than that, SSIM will present a rough measure of the shortfall if 
the transcode isn't lossless. A framehash is either a match or not.


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


Re: [FFmpeg-devel] [PATCH] amfenc: Add support for pict_type field

2019-02-10 Thread Mark Thompson
On 04/02/2019 14:41, Michael Dirks wrote:
> On 04.02.2019 11:05, Mark Thompson wrote:
>> Can you explain what this "skip frame" actually does in the encoder?  The 
>> concept does not exist in H.264 or H.265, as far as I know.
> 
> I believe this has to do with the pic_struct flag which has a value of 7 for 
> frame doubling and 8 for frame tripling, see page 345 (PDF page 367) Table 
> D-1 Interpretation of pic_struct in Rec. ITU-T H.264 (04/2017). However I do 
> not work for AMD (and their encoder is closed source and a piece of 
> hardware), so I can't confirm that this is actually the behavior, nor do I 
> have any tools that can read and show all H.264 headers. An alternative would 
> be that AMDs encoder is instead choosing to emit a frame that has maximum 
> compression and references the previous frame for all data, thus causing a 
> copy of it.

You can see this with the trace_headers bitstream filter.  (Try "-c:v h264_amf 
-options... -bsf:v trace_headers -f null -", or it can be used with streamcopy 
to examine an existing file.)

>>> +    case AV_PICTURE_TYPE_I:
>>> +    AMF_ASSIGN_PROPERTY_INT64(res, surface, 
>>> AMF_VIDEO_ENCODER_FORCE_PICTURE_TYPE, AMF_VIDEO_ENCODER_PICTURE_TYPE_I);
>> Consider whether you need to set IDR here rather than I, and maybe add a 
>> comment explaining the choice?  The normal use of this is to indicate that 
>> an IRAP should be generated, not just a single intra picture.  (Compare 
>> libx264, which has an additional flag controlling whether the forced picture 
>> is IDR or I, but I believe it is still always an IRAP there.)
>>> +    // Keyframe overrides previous assignment.
>>> +    if (frame->key_frame) {
>> This flag shouldn't be read by encoders.  (I believe it is set by the 
>> decoder and never cleared, so with this test you will have surprising 
>> behaviour where the output stream gets key frames everywhere that the input 
>> stream had them, in addition to those dictated by its own GOP structure.)
> I went by the documentation in the individual header files, which does not 
> actually claim key_frame to be a decoder only field (libavutil/frame.h):
> 
>> /**
>> * 1 -> keyframe, 0-> not
>> */
>>    int key_frame;
> 
> Therefore this patch uses the field exactly as it is documented in the 
> frame.h file, and also treats AV_PICTURE_TYPE_I as a request for an Intra 
> Picture instead of an IDR Picture.

Well yes, but that's not actually going to work - try it with a set of JPEGs as 
input and you'll observe the problem.  (Note that no other encoder uses this 
field on input, though some do use it internally.)

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


Re: [FFmpeg-devel] Proposal: Homebrew tap for FFmpeg

2019-02-10 Thread Werner Robitza
On Sat, Feb 9, 2019 at 11:59 PM Jean-Baptiste Kempf  wrote:
>
> On Sat, 9 Feb 2019, at 11:59, Werner Robitza wrote:
> > Then the only consequence can be to remove these options or support
> > for these libraries altogether, because you'll find plenty of guides
> > and recommendations on how to build ffmpeg with non-free libs on the
> > Internet – even supplied by members who are very active in the FFmpeg
> > community. It is certainly your prerogative to be against explicit
> > advertising, but where do you draw the line? Has there been any
> > precedent with this, or is this going to be decided on a case-by-case
> > basis?
> >
> > The only consequence would be a formula that is not owned and
> > controlled by FFmpeg, and people will continue to build non-free
> > binaries.
>
> But then, it is not the project, doing it, but someone else.

The project won't be building non-free binaries and ship those to
people. It's the users who will do it on their machine.

> To come back to the main topic, you can have a full FFmpeg in homebrew with 
> all the libraries activated by default, if you want, without any issue.

No, that's not the case.

> I therefore do not see at all the need for options.

They are needed since not everyone wants or needs a full-featured
ffmpeg with all third-party libraries. There can be sane defaults,
like there have been for years, and some libraries can be enabled
optionally.

> Those options are just for non-free cases, and to be honest, I don't see why 
> FFmpeg should advertise those.

That is not correct. The following options/dependencies are not
present in Homebrew core:

chromaprint, fdk-aac, game-music-emu, libbs2b, libcaca, libgsm,
libmodplug, librsvg, libssh, libvidstab, libvmaf, openh264, openssl,
srt, two-lame, wavpack, webp, zeromq, zimg
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] Proposal: Homebrew tap for FFmpeg

2019-02-10 Thread Jean-Baptiste Kempf
On Sun, 10 Feb 2019, at 19:37, Werner Robitza wrote:
> > Those options are just for non-free cases, and to be honest, I don't see 
> > why FFmpeg should advertise those.
> 
> That is not correct. The following options/dependencies are not
> present in Homebrew core:
> 
> chromaprint, fdk-aac, game-music-emu, libbs2b, libcaca, libgsm,
> libmodplug, librsvg, libssh, libvidstab, libvmaf, openh264, openssl,
> srt, two-lame, wavpack, webp, zeromq, zimg

Fair enough.
I still object to an official recipe activating non-free options.


-- 
Jean-Baptiste Kempf -  President
+33 672 704 734
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH v2 08/11] vaapi_encode_mjpeg: Warn if input is not full range

2019-02-10 Thread Carl Eugen Hoyos
2019-02-10 19:21 GMT+01:00, Mark Thompson :
> On 05/02/2019 13:27, Carl Eugen Hoyos wrote:
>> 2019-01-28 0:47 GMT+01:00, Mark Thompson :
>>
>>> +if (avctx->color_range == AVCOL_RANGE_MPEG) {
>>> +av_log(avctx, AV_LOG_WARNING, "Input video does not appear "
>>> +   "to use full-range: output colours may be incorrect.\n");
>>
>> The wording seems not ideal to me:
>> Are you not sure if the encoder will produce correct output for
>> mpeg-range jpeg?
>> That's how it sounds to me...
>
> The problem is that, unlike H.264 and other codecs, JPEG has no way to
> signal that the data are actually limited-range.  The encode will preserve
> the values, but those values will not have the right interpretation at a
> remote end which only looks at the JPEG data.  On the other hand, if
> signalled by some out-of-band method (perhaps the container, or you control
> both ends) everything is perfectly usable.
>
> I agree the message isn't particularly clear; do you have any thoughts about
> how it should be phrased?

Can't you add the same COM tag as mjpegenc_common.c does?

If not it could be something like "mpeg-range jpeg will be undetectable".

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


Re: [FFmpeg-devel] Reimbursement request

2019-02-10 Thread Thilo Borgmann
Hi,

> I'm requesting the reimbursement of travel expenses for the Google
> Mentor Summit.
> ...

a little late, however I'd also like to request reimbursement for my travel 
there last October [1].

Total cost of my flight: 1348.33€

Will send the paperworks to Stefano as usual.

Thanks,
Thilo


[1] https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2018-October/235353.html
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] Reimbursement request for LDP

2019-02-10 Thread Thilo Borgmann
Hi,

I'd like to request reimbursement for my travel to the LDP [1].

Accomodation was sponsored by the LDP team, so travel costs are rather low: 
21.90€

Will send the paperwork to Stefano.

Thanks,
Thilo


[1] https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2018-October/235352.html
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] lavc/libvpxenc: Deprecate lossless option

2019-02-10 Thread Carl Eugen Hoyos
2019-02-10 5:44 GMT+01:00, Gyan :
>
>
> On 09-02-2019 04:45 PM, Carl Eugen Hoyos wrote:
>> 2019-02-09 7:49 GMT+01:00, Gyan :
>>>
>>> On 09-02-2019 02:26 AM, Carl Eugen Hoyos wrote:
 2019-02-08 6:08 GMT+01:00, Gyan :
> On 08-02-2019 03:31 AM, Carl Eugen Hoyos wrote:
>> .
>> No strong opinion here, I hadn't realized that -crf 0 only works with
>> newer versions.
>>
>> Can you explain why crf alone has no effect and needs -b:v 0?
>> Isn't this a bug both with libvpx and libaom?
>>
> With crf present but VBV params absent, VPX operates using a
> constrained
> Q RC mode , where the target bitrate acts as a ceiling. Since acvodec
> has a non-zero default -b of 200 kbps, this produces undesirable
> effects. If set to 0, VPX switches to constant quality.
 Yes.
 This looks like a bug to me.
>>> If there's a bug, it's in libav, in that, we can't signal when a value
>>> is assigned by the user or via user-initiated logic, versus being
>>> assigned as a default fallback. So it's a design conflict, and the
>>> workaround has been long documented.
>>>
>>> Maybe just before the next major bump, a new field should be
>>> introduced in AVOptions which registers if the field was
>>> populated at the behest of the user.
>> We could set the default for "b" to "0" without a major bump.
> Do you mean just the defaults in the vpx wrapper?

Yes.

> Ok.  Changing the lib
> default will affect encoders which make use the bitrate value, like mjpeg.
>
>> Is it expected that constant crf with vp8 leads to broken output?
> Not broken, simply unexpected for those used to  how x264/5 handle it in
> ffmpeg.

I tested -crf with "b:v 0" and the vp8 output (current FFmpeg git head,
a libvpx release) was broken: Every 100th or so frame looked ugly
and very different from the surrounding frames (no scene change).

I will try to reproduce again.

> I do see this block though,
>
>if (avctx->codec_id == AV_CODEC_ID_VP9 && ctx->lossless == 1) {
>enccfg.rc_min_quantizer =
>enccfg.rc_max_quantizer = 0;
>} else {
>if (avctx->qmin >= 0)
>enccfg.rc_min_quantizer = avctx->qmin;
>if (avctx->qmax >= 0)
>enccfg.rc_max_quantizer = avctx->qmax;
>}
>
>
> Looks like the quantizer range is disabled only by using the deprecated
> option, or has this changed?
 Is your question "Isn't 'lossless 1' necessary for lossless encoding"?
>>> Yes. A quick glance at vpx HEAD indicates it is, although
>>> -qmin & -qmax 0 should also work.
>>
> Also, with libvpx v1.7.0-1758, I get different results for -crf 0 -b:v
> 0  vs only -lossless 1, with the latter producing a slightly larger
> file, and its result showing a slightly larger SSIM score. Although
> neither produces a SSIM of 1, like libx264. Not truly lossless?
 Please provide your input sample.
>>> I can reproduce the difference in result with
>>> fate-suite/h264-conformance/src19td.IBP.264
>> Yes, this returns a few non-lossless frames.
>>
>> Is the encoder meant to be used for actual encoding?
>> Or does -crf 0 have several meanings depending on specific options
>> and vp8 is just bad?

This should be vp9 (or libvpx assuming the above)

> We're talking about VP9. VP8 has no lossless mode. Yes, this is a
> production encoder - Youtube uses it to generate VP9 streams for
> uploaded videos.
>
> Like I mentioned earlier, CRF +  bitrate triggers constrained quality.
> If the rate ceiling is high enough, the result is equivalent to
> unclamped CRF, which is what the wrapper does when avctx bitrate is 0,
> set its own bitrate target to 1 Gbps.

So the problem is that with b:v 0 you currently get a completely unexpected
output depending on the resolution of the video? (And if you did use another
value for crf than 0, you won't even have a possibility to find out.)
Wow...

> In any case, -lossless shouldn't be deprecated unless the workaround
> produces identical results.

My question here is why crf 0 does not produce lossless encodings
(with "b:v 0"). Is this expected for some reason?
(But I get the point now after reading above sentence again.)

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


Re: [FFmpeg-devel] lavc/libvpxenc: Deprecate lossless option

2019-02-10 Thread Carl Eugen Hoyos
2019-02-10 13:06 GMT+01:00, Kieran O Leary :
> On Sat, 9 Feb 2019, 06:49 Gyan > although the `-lossless 1` encoding does return as lossless in SSIM.
>
> What ssim command did you use, and why use this over a hash muxer like
> framehash? I'm always on the lookout for losslessness verification methods.

If you trust FFmpeg (this thread gives several indications why you shouldn't)
the output format crc is simpler to type, easier to interpret and - in theory -
safer than ssim.

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


Re: [FFmpeg-devel] Reimbursement request

2019-02-10 Thread Paul B Mahol
On 2/10/19, Thilo Borgmann  wrote:
> Hi,
>
>> I'm requesting the reimbursement of travel expenses for the Google
>> Mentor Summit.
>> ...
>
> a little late, however I'd also like to request reimbursement for my travel
> there last October [1].
>
> Total cost of my flight: 1348.33€

Have picture of it?

>
> Will send the paperworks to Stefano as usual.
>
> Thanks,
> Thilo
>
>
> [1] https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2018-October/235353.html
> ___
> 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] Reimbursement request

2019-02-10 Thread Thilo Borgmann
Am 10.02.19 um 19:58 schrieb Paul B Mahol:
> On 2/10/19, Thilo Borgmann  wrote:
>> Hi,
>>
>>> I'm requesting the reimbursement of travel expenses for the Google
>>> Mentor Summit.
>>> ...
>>
>> a little late, however I'd also like to request reimbursement for my travel
>> there last October [1].
>>
>> Total cost of my flight: 1348.33€
> 
> Have picture of it?

Send to Stefano. Why?


>>
>> Will send the paperworks to Stefano as usual.
>>
>> Thanks,
>> Thilo
>>
>>
>> [1] https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2018-October/235353.html
>> ___
>> 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 mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] Reimbursement request

2019-02-10 Thread Thilo Borgmann
Am 10.02.19 um 20:09 schrieb Paul B Mahol:
> On 2/10/19, Thilo Borgmann  wrote:
>> Am 10.02.19 um 19:58 schrieb Paul B Mahol:
>>> On 2/10/19, Thilo Borgmann  wrote:
 Hi,

> I'm requesting the reimbursement of travel expenses for the Google
> Mentor Summit.
> ...

 a little late, however I'd also like to request reimbursement for my
 travel
 there last October [1].

 Total cost of my flight: 1348.33€
>>>
>>> Have picture of it?
>>
>> Send to Stefano. Why?
>>
> 
> Just asked.

Okay. Want to see it yourself?



 Will send the paperworks to Stefano as usual.

 Thanks,
 Thilo


 [1]
 https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2018-October/235353.html
 ___
 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 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 mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] Reimbursement request

2019-02-10 Thread Paul B Mahol
On 2/10/19, Thilo Borgmann  wrote:
> Am 10.02.19 um 19:58 schrieb Paul B Mahol:
>> On 2/10/19, Thilo Borgmann  wrote:
>>> Hi,
>>>
 I'm requesting the reimbursement of travel expenses for the Google
 Mentor Summit.
 ...
>>>
>>> a little late, however I'd also like to request reimbursement for my
>>> travel
>>> there last October [1].
>>>
>>> Total cost of my flight: 1348.33€
>>
>> Have picture of it?
>
> Send to Stefano. Why?
>

Just asked.

>
>>>
>>> Will send the paperworks to Stefano as usual.
>>>
>>> Thanks,
>>> Thilo
>>>
>>>
>>> [1]
>>> https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2018-October/235353.html
>>> ___
>>> 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 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 v2 08/11] vaapi_encode_mjpeg: Warn if input is not full range

2019-02-10 Thread Mark Thompson
On 10/02/2019 18:49, Carl Eugen Hoyos wrote:
> 2019-02-10 19:21 GMT+01:00, Mark Thompson :
>> On 05/02/2019 13:27, Carl Eugen Hoyos wrote:
>>> 2019-01-28 0:47 GMT+01:00, Mark Thompson :
>>>
 +if (avctx->color_range == AVCOL_RANGE_MPEG) {
 +av_log(avctx, AV_LOG_WARNING, "Input video does not appear "
 +   "to use full-range: output colours may be incorrect.\n");
>>>
>>> The wording seems not ideal to me:
>>> Are you not sure if the encoder will produce correct output for
>>> mpeg-range jpeg?
>>> That's how it sounds to me...
>>
>> The problem is that, unlike H.264 and other codecs, JPEG has no way to
>> signal that the data are actually limited-range.  The encode will preserve
>> the values, but those values will not have the right interpretation at a
>> remote end which only looks at the JPEG data.  On the other hand, if
>> signalled by some out-of-band method (perhaps the container, or you control
>> both ends) everything is perfectly usable.
>>
>> I agree the message isn't particularly clear; do you have any thoughts about
>> how it should be phrased?
> 
> Can't you add the same COM tag as mjpegenc_common.c does?

Hmm, I had not really thought about that.  Do you know any reference for where 
this feature comes from and what might support it?

It was added as dd4f8a04 in 2005, but the motivation for the change isn't 
recorded.  Searching reveals little beyond references to the FFmpeg source code.

Thanks,

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


Re: [FFmpeg-devel] Reimbursement request

2019-02-10 Thread Paul B Mahol
On 2/10/19, Thilo Borgmann  wrote:
> Am 10.02.19 um 20:09 schrieb Paul B Mahol:
>> On 2/10/19, Thilo Borgmann  wrote:
>>> Am 10.02.19 um 19:58 schrieb Paul B Mahol:
 On 2/10/19, Thilo Borgmann  wrote:
> Hi,
>
>> I'm requesting the reimbursement of travel expenses for the Google
>> Mentor Summit.
>> ...
>
> a little late, however I'd also like to request reimbursement for my
> travel
> there last October [1].
>
> Total cost of my flight: 1348.33€

 Have picture of it?
>>>
>>> Send to Stefano. Why?
>>>
>>
>> Just asked.
>
> Okay. Want to see it yourself?
>

No, I trust Stefano.

>
>
> Will send the paperworks to Stefano as usual.
>
> Thanks,
> Thilo
>
>
> [1]
> https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2018-October/235353.html
> ___
> 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 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 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 v3 01/12] vaapi_encode: Support more RC modes

2019-02-10 Thread Mark Thompson
Allow setting the mode explicitly, and try to make a sensible choice
given the available parameters if not.
---
 doc/encoders.texi |  24 +++
 libavcodec/vaapi_encode.c | 382 +++---
 libavcodec/vaapi_encode.h |  65 +++
 3 files changed, 358 insertions(+), 113 deletions(-)

diff --git a/doc/encoders.texi b/doc/encoders.texi
index e86ae69cc5..29625ba07c 100644
--- a/doc/encoders.texi
+++ b/doc/encoders.texi
@@ -2824,6 +2824,30 @@ Set the B-frame reference depth.  When set to one (the 
default), all B-frames
 will refer only to P- or I-frames.  When set to greater values multiple layers
 of B-frames will be present, frames in each layer only referring to frames in
 higher layers.
+
+@item rc_mode
+Set the rate control mode to use.  A given driver may only support a subset of
+modes.
+
+Possible modes:
+@table @option
+@item auto
+Choose the mode automatically based on driver support and the other options.
+This is the default.
+@item CQP
+Constant-quality.
+@item CBR
+Constant-bitrate.
+@item VBR
+Variable-bitrate.
+@item ICQ
+Intelligent constant-quality.
+@item QVBR
+Quality-defined variable-bitrate.
+@item AVBR
+Average variable bitrate.
+@end table
+
 @end table
 
 Each encoder also has its own specific options:
diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
index b4e9fadaee..2dda451882 100644
--- a/libavcodec/vaapi_encode.c
+++ b/libavcodec/vaapi_encode.c
@@ -1283,17 +1283,42 @@ fail:
 return err;
 }
 
+static const VAAPIEncodeRCMode vaapi_encode_rc_modes[] = {
+//  Bitrate   Quality
+// | Maxrate | HRD/VBV
+{ 0 }, //  ||||
+{ RC_MODE_CQP,  "CQP",  1, VA_RC_CQP,  0,   0,   1,   0 },
+{ RC_MODE_CBR,  "CBR",  1, VA_RC_CBR,  1,   0,   0,   1 },
+{ RC_MODE_VBR,  "VBR",  1, VA_RC_VBR,  1,   1,   0,   1 },
+#if VA_CHECK_VERSION(1, 1, 0)
+{ RC_MODE_ICQ,  "ICQ",  1, VA_RC_ICQ,  0,   0,   1,   0 },
+#else
+{ RC_MODE_ICQ,  "ICQ",  0 },
+#endif
+#if VA_CHECK_VERSION(1, 3, 0)
+{ RC_MODE_QVBR, "QVBR", 1, VA_RC_QVBR, 1,   1,   1,   1 },
+{ RC_MODE_AVBR, "AVBR", 0, VA_RC_AVBR, 1,   0,   0,   0 },
+#else
+{ RC_MODE_QVBR, "QVBR", 0 },
+{ RC_MODE_AVBR, "AVBR", 0 },
+#endif
+};
+
 static av_cold int vaapi_encode_init_rate_control(AVCodecContext *avctx)
 {
 VAAPIEncodeContext *ctx = avctx->priv_data;
+uint32_t supported_va_rc_modes;
+const VAAPIEncodeRCMode *rc_mode;
 int64_t rc_bits_per_second;
 int rc_target_percentage;
 int rc_window_size;
+int rc_quality;
 int64_t hrd_buffer_size;
 int64_t hrd_initial_buffer_fullness;
 int fr_num, fr_den;
 VAConfigAttrib rc_attr = { VAConfigAttribRateControl };
 VAStatus vas;
+char supported_rc_modes_string[64];
 
 vas = vaGetConfigAttributes(ctx->hwctx->display,
 ctx->va_profile, ctx->va_entrypoint,
@@ -1303,119 +1328,213 @@ static av_cold int 
vaapi_encode_init_rate_control(AVCodecContext *avctx)
"config attribute: %d (%s).\n", vas, vaErrorStr(vas));
 return AVERROR_EXTERNAL;
 }
-
 if (rc_attr.value == VA_ATTRIB_NOT_SUPPORTED) {
 av_log(avctx, AV_LOG_VERBOSE, "Driver does not report any "
-   "supported rate control modes: assuming constant-quality.\n");
-ctx->va_rc_mode = VA_RC_CQP;
-return 0;
-}
-if (ctx->codec->flags & FLAG_CONSTANT_QUALITY_ONLY ||
-avctx->flags & AV_CODEC_FLAG_QSCALE ||
-avctx->bit_rate <= 0) {
-if (rc_attr.value & VA_RC_CQP) {
-av_log(avctx, AV_LOG_VERBOSE, "Using constant-quality mode.\n");
-ctx->va_rc_mode = VA_RC_CQP;
-if (avctx->bit_rate > 0 || avctx->rc_max_rate > 0) {
-av_log(avctx, AV_LOG_WARNING, "Bitrate target parameters "
-   "ignored in constant-quality mode.\n");
+   "supported rate control modes: assuming CQP only.\n");
+supported_va_rc_modes = VA_RC_CQP;
+strcpy(supported_rc_modes_string, "unknown");
+} else {
+char *str = supported_rc_modes_string;
+size_t len = sizeof(supported_rc_modes_string);
+int i, first = 1, res;
+
+supported_va_rc_modes = rc_attr.value;
+for (i = 0; i < FF_ARRAY_ELEMS(vaapi_encode_rc_modes); i++) {
+rc_mode = &vaapi_encode_rc_modes[i];
+if (supported_va_rc_modes & rc_mode->va_mode) {
+res = snprintf(str, len, "%s%s",
+   first ? "" : ", ", rc_mode->name);
+first = 0;
+if (res < 0) {
+*str = 0;
+break;
+}
+len -= res;
+str += res;
+if (len == 0)
+break;
 }
-return 0;
-} else {
-av_log(avctx, AV_LOG_ERROR,

[FFmpeg-devel] [PATCH v3 02/12] vaapi_encode_h264: Enable support for more RC modes

2019-02-10 Thread Mark Thompson
Also fixes QP going out of range when modified by the quant factor/offset
values.
---
 libavcodec/vaapi_encode_h264.c | 31 +++
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/libavcodec/vaapi_encode_h264.c b/libavcodec/vaapi_encode_h264.c
index 4ea62d96f3..fb55eb7779 100644
--- a/libavcodec/vaapi_encode_h264.c
+++ b/libavcodec/vaapi_encode_h264.c
@@ -1071,33 +1071,34 @@ static av_cold int 
vaapi_encode_h264_configure(AVCodecContext *avctx)
 priv->mb_height = FFALIGN(avctx->height, 16) / 16;
 
 if (ctx->va_rc_mode == VA_RC_CQP) {
-priv->fixed_qp_p = priv->qp;
+priv->fixed_qp_p = av_clip(ctx->rc_quality, 1, 51);
 if (avctx->i_quant_factor > 0.0)
-priv->fixed_qp_idr = (int)((priv->fixed_qp_p * 
avctx->i_quant_factor +
-avctx->i_quant_offset) + 0.5);
+priv->fixed_qp_idr =
+av_clip((avctx->i_quant_factor * priv->fixed_qp_p +
+ avctx->i_quant_offset) + 0.5, 1, 51);
 else
 priv->fixed_qp_idr = priv->fixed_qp_p;
 if (avctx->b_quant_factor > 0.0)
-priv->fixed_qp_b = (int)((priv->fixed_qp_p * avctx->b_quant_factor 
+
-  avctx->b_quant_offset) + 0.5);
+priv->fixed_qp_b =
+av_clip((avctx->b_quant_factor * priv->fixed_qp_p +
+ avctx->b_quant_offset) + 0.5, 1, 51);
 else
 priv->fixed_qp_b = priv->fixed_qp_p;
 
-priv->sei &= ~SEI_TIMING;
-
 av_log(avctx, AV_LOG_DEBUG, "Using fixed QP = "
"%d / %d / %d for IDR- / P- / B-frames.\n",
priv->fixed_qp_idr, priv->fixed_qp_p, priv->fixed_qp_b);
 
-} else if (ctx->va_rc_mode == VA_RC_CBR ||
-   ctx->va_rc_mode == VA_RC_VBR) {
+} else {
 // These still need to be  set for pic_init_qp/slice_qp_delta.
 priv->fixed_qp_idr = 26;
 priv->fixed_qp_p   = 26;
 priv->fixed_qp_b   = 26;
+}
 
-} else {
-av_assert0(0 && "Invalid RC mode.");
+if (!ctx->rc_mode->hrd) {
+// Timing SEI requires a mode respecting HRD parameters.
+priv->sei &= ~SEI_TIMING;
 }
 
 if (priv->sei & SEI_IDENTIFIER) {
@@ -1147,6 +1148,8 @@ static const VAAPIEncodeType vaapi_encode_type_h264 = {
  FLAG_B_PICTURE_REFERENCES |
  FLAG_NON_IDR_KEY_PICTURES,
 
+.default_quality   = 20,
+
 .configure = &vaapi_encode_h264_configure,
 
 .picture_priv_data_size = sizeof(VAAPIEncodeH264Picture),
@@ -1226,6 +1229,9 @@ static av_cold int vaapi_encode_h264_init(AVCodecContext 
*avctx)
 
 ctx->slice_block_height = ctx->slice_block_width = 16;
 
+if (priv->qp > 0)
+ctx->explicit_qp = priv->qp;
+
 return ff_vaapi_encode_init(avctx);
 }
 
@@ -1243,9 +1249,10 @@ static av_cold int 
vaapi_encode_h264_close(AVCodecContext *avctx)
 #define FLAGS (AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_ENCODING_PARAM)
 static const AVOption vaapi_encode_h264_options[] = {
 VAAPI_ENCODE_COMMON_OPTIONS,
+VAAPI_ENCODE_RC_OPTIONS,
 
 { "qp", "Constant QP (for P-frames; scaled by qfactor/qoffset for I/B)",
-  OFFSET(qp), AV_OPT_TYPE_INT, { .i64 = 20 }, 0, 52, FLAGS },
+  OFFSET(qp), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 52, FLAGS },
 { "quality", "Set encode quality (trades off against speed, higher is 
faster)",
   OFFSET(quality), AV_OPT_TYPE_INT, { .i64 = -1 }, -1, INT_MAX, FLAGS },
 { "coder", "Entropy coder type",
-- 
2.19.2

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


[FFmpeg-devel] [PATCH v3 05/12] vaapi_encode_vp8: Enable support for more RC modes

2019-02-10 Thread Mark Thompson
---
 libavcodec/vaapi_encode_vp8.c | 16 ++--
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/libavcodec/vaapi_encode_vp8.c b/libavcodec/vaapi_encode_vp8.c
index 166636cd84..ddbe4c9075 100644
--- a/libavcodec/vaapi_encode_vp8.c
+++ b/libavcodec/vaapi_encode_vp8.c
@@ -161,14 +161,15 @@ static int 
vaapi_encode_vp8_write_quant_table(AVCodecContext *avctx,
 
 static av_cold int vaapi_encode_vp8_configure(AVCodecContext *avctx)
 {
+VAAPIEncodeContext *ctx = avctx->priv_data;
 VAAPIEncodeVP8Context *priv = avctx->priv_data;
 
-priv->q_index_p = av_clip(avctx->global_quality, 0, VP8_MAX_QUANT);
+priv->q_index_p = av_clip(ctx->rc_quality, 0, VP8_MAX_QUANT);
 if (avctx->i_quant_factor > 0.0)
-priv->q_index_i = av_clip((avctx->global_quality *
-   avctx->i_quant_factor +
-   avctx->i_quant_offset) + 0.5,
-  0, VP8_MAX_QUANT);
+priv->q_index_i =
+av_clip((avctx->i_quant_factor * priv->q_index_p  +
+ avctx->i_quant_offset) + 0.5,
+0, VP8_MAX_QUANT);
 else
 priv->q_index_i = priv->q_index_p;
 
@@ -185,6 +186,8 @@ static const VAAPIEncodeType vaapi_encode_type_vp8 = {
 
 .configure = &vaapi_encode_vp8_configure,
 
+.default_quality   = 40,
+
 .sequence_params_size  = sizeof(VAEncSequenceParameterBufferVP8),
 .init_sequence_params  = &vaapi_encode_vp8_init_sequence_params,
 
@@ -215,6 +218,8 @@ static av_cold int vaapi_encode_vp8_init(AVCodecContext 
*avctx)
 #define FLAGS (AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_ENCODING_PARAM)
 static const AVOption vaapi_encode_vp8_options[] = {
 VAAPI_ENCODE_COMMON_OPTIONS,
+VAAPI_ENCODE_RC_OPTIONS,
+
 { "loop_filter_level", "Loop filter level",
   OFFSET(loop_filter_level), AV_OPT_TYPE_INT, { .i64 = 16 }, 0, 63, FLAGS 
},
 { "loop_filter_sharpness", "Loop filter sharpness",
@@ -226,7 +231,6 @@ static const AVCodecDefault vaapi_encode_vp8_defaults[] = {
 { "b",  "0"   },
 { "bf", "0"   },
 { "g",  "120" },
-{ "global_quality", "40"  },
 { "qmin",   "-1"  },
 { "qmax",   "-1"  },
 { NULL },
-- 
2.19.2

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


[FFmpeg-devel] [PATCH v3 11/12] vaapi_encode_h264: Don't include AUD with slice header

2019-02-10 Thread Mark Thompson
Always write it as a RawData block, even if there is no SEI as well.
---
 libavcodec/vaapi_encode_h264.c | 40 +++---
 1 file changed, 17 insertions(+), 23 deletions(-)

diff --git a/libavcodec/vaapi_encode_h264.c b/libavcodec/vaapi_encode_h264.c
index fb55eb7779..b65ee4bec0 100644
--- a/libavcodec/vaapi_encode_h264.c
+++ b/libavcodec/vaapi_encode_h264.c
@@ -187,13 +187,6 @@ static int 
vaapi_encode_h264_write_slice_header(AVCodecContext *avctx,
 CodedBitstreamFragment   *au = &priv->current_access_unit;
 int err;
 
-if (priv->aud_needed) {
-err = vaapi_encode_h264_add_nal(avctx, au, &priv->raw_aud);
-if (err < 0)
-goto fail;
-priv->aud_needed = 0;
-}
-
 err = vaapi_encode_h264_add_nal(avctx, au, &priv->raw_slice);
 if (err < 0)
 goto fail;
@@ -213,16 +206,16 @@ static int 
vaapi_encode_h264_write_extra_header(AVCodecContext *avctx,
 CodedBitstreamFragment   *au = &priv->current_access_unit;
 int err, i;
 
+if (priv->aud_needed) {
+err = vaapi_encode_h264_add_nal(avctx, au, &priv->raw_aud);
+if (err < 0)
+goto fail;
+priv->aud_needed = 0;
+}
+
 if (priv->sei_needed) {
 H264RawSEI *sei = &priv->raw_sei;
 
-if (priv->aud_needed) {
-err = vaapi_encode_h264_add_nal(avctx, au, &priv->raw_aud);
-if (err < 0)
-goto fail;
-priv->aud_needed = 0;
-}
-
 *sei = (H264RawSEI) {
 .nal_unit_header = {
 .nal_unit_type = H264_NAL_SEI,
@@ -260,15 +253,6 @@ static int 
vaapi_encode_h264_write_extra_header(AVCodecContext *avctx,
 goto fail;
 priv->sei_needed = 0;
 
-err = vaapi_encode_h264_write_access_unit(avctx, data, data_len, au);
-if (err < 0)
-goto fail;
-
-ff_cbs_fragment_uninit(priv->cbc, au);
-
-*type = VAEncPackedHeaderRawData;
-return 0;
-
 #if !CONFIG_VAAPI_1
 } else if (priv->sei_cbr_workaround_needed) {
 // Insert a zero-length header using the old SEI type.  This is
@@ -285,6 +269,16 @@ static int 
vaapi_encode_h264_write_extra_header(AVCodecContext *avctx,
 return AVERROR_EOF;
 }
 
+
+err = vaapi_encode_h264_write_access_unit(avctx, data, data_len, au);
+if (err < 0)
+goto fail;
+
+ff_cbs_fragment_uninit(priv->cbc, au);
+
+*type = VAEncPackedHeaderRawData;
+return 0;
+
 fail:
 ff_cbs_fragment_uninit(priv->cbc, au);
 return err;
-- 
2.19.2

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


[FFmpeg-devel] [PATCH v3 03/12] vaapi_encode_h265: Enable support for more RC modes

2019-02-10 Thread Mark Thompson
Also fixes QP going out of range when modified by the quant factor/offset
values, and clarifies the QP behaviour for >8-bit modes.
---
 libavcodec/vaapi_encode_h265.c | 32 
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/libavcodec/vaapi_encode_h265.c b/libavcodec/vaapi_encode_h265.c
index 19e7104e9e..4d2a64308e 100644
--- a/libavcodec/vaapi_encode_h265.c
+++ b/libavcodec/vaapi_encode_h265.c
@@ -1076,15 +1076,21 @@ static av_cold int 
vaapi_encode_h265_configure(AVCodecContext *avctx)
 return err;
 
 if (ctx->va_rc_mode == VA_RC_CQP) {
-priv->fixed_qp_p = priv->qp;
+// Note that VAAPI only supports positive QP values - the range is
+// therefore always bounded below by 1, even in 10-bit mode where
+// it should go down to -12.
+
+priv->fixed_qp_p = av_clip(ctx->rc_quality, 1, 51);
 if (avctx->i_quant_factor > 0.0)
-priv->fixed_qp_idr = (int)((priv->fixed_qp_p * 
avctx->i_quant_factor +
-avctx->i_quant_offset) + 0.5);
+priv->fixed_qp_idr =
+av_clip((avctx->i_quant_factor * priv->fixed_qp_p +
+ avctx->i_quant_offset) + 0.5, 1, 51);
 else
 priv->fixed_qp_idr = priv->fixed_qp_p;
 if (avctx->b_quant_factor > 0.0)
-priv->fixed_qp_b = (int)((priv->fixed_qp_p * avctx->b_quant_factor 
+
-  avctx->b_quant_offset) + 0.5);
+priv->fixed_qp_b =
+av_clip((avctx->b_quant_factor * priv->fixed_qp_p +
+ avctx->b_quant_offset) + 0.5, 1, 51);
 else
 priv->fixed_qp_b = priv->fixed_qp_p;
 
@@ -1092,15 +1098,11 @@ static av_cold int 
vaapi_encode_h265_configure(AVCodecContext *avctx)
"%d / %d / %d for IDR- / P- / B-frames.\n",
priv->fixed_qp_idr, priv->fixed_qp_p, priv->fixed_qp_b);
 
-} else if (ctx->va_rc_mode == VA_RC_CBR ||
-   ctx->va_rc_mode == VA_RC_VBR) {
-// These still need to be  set for pic_init_qp/slice_qp_delta.
+} else {
+// These still need to be set for init_qp/slice_qp_delta.
 priv->fixed_qp_idr = 30;
 priv->fixed_qp_p   = 30;
 priv->fixed_qp_b   = 30;
-
-} else {
-av_assert0(0 && "Invalid RC mode.");
 }
 
 return 0;
@@ -1124,6 +1126,8 @@ static const VAAPIEncodeType vaapi_encode_type_h265 = {
  FLAG_B_PICTURE_REFERENCES |
  FLAG_NON_IDR_KEY_PICTURES,
 
+.default_quality   = 25,
+
 .configure = &vaapi_encode_h265_configure,
 
 .picture_priv_data_size = sizeof(VAAPIEncodeH265Picture),
@@ -1175,6 +1179,9 @@ static av_cold int vaapi_encode_h265_init(AVCodecContext 
*avctx)
 // CTU size is currently hard-coded to 32.
 ctx->slice_block_width = ctx->slice_block_height = 32;
 
+if (priv->qp > 0)
+ctx->explicit_qp = priv->qp;
+
 return ff_vaapi_encode_init(avctx);
 }
 
@@ -1191,9 +1198,10 @@ static av_cold int 
vaapi_encode_h265_close(AVCodecContext *avctx)
 #define FLAGS (AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_ENCODING_PARAM)
 static const AVOption vaapi_encode_h265_options[] = {
 VAAPI_ENCODE_COMMON_OPTIONS,
+VAAPI_ENCODE_RC_OPTIONS,
 
 { "qp", "Constant QP (for P-frames; scaled by qfactor/qoffset for I/B)",
-  OFFSET(qp), AV_OPT_TYPE_INT, { .i64 = 25 }, 0, 52, FLAGS },
+  OFFSET(qp), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 52, FLAGS },
 
 { "aud", "Include AUD",
   OFFSET(aud), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, FLAGS },
-- 
2.19.2

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


[FFmpeg-devel] [PATCH v3 10/12] vaapi_encode: Add support for VFR mode

2019-02-10 Thread Mark Thompson
Use the frame-skip feature to maintain a specified framerate from the
point of view of the driver.
---
"max_fps" collides with an option used in lavf, so renamed to "vfr_max_fps" 
here.  (That hopefully also makes it clearer what the use of the option is.)


 doc/encoders.texi |   7 +++
 libavcodec/vaapi_encode.c | 116 +++---
 libavcodec/vaapi_encode.h |  18 +-
 3 files changed, 132 insertions(+), 9 deletions(-)

diff --git a/doc/encoders.texi b/doc/encoders.texi
index 29625ba07c..39b8fc1d37 100644
--- a/doc/encoders.texi
+++ b/doc/encoders.texi
@@ -2848,6 +2848,13 @@ Quality-defined variable-bitrate.
 Average variable bitrate.
 @end table
 
+@item vfr_max_fps
+Enable VFR encoding with this maximum framerate.  This is implemented by
+indicating to the driver that frames have been skipped at some locations in
+the stream, and requires driver support.  The minimum interval between frames
+must not be smaller than this, and there may be problems if the maximum
+interval is more than a small multiple of it.
+
 @end table
 
 Each encoder also has its own specific options:
diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
index 2dda451882..df8f65e431 100644
--- a/libavcodec/vaapi_encode.c
+++ b/libavcodec/vaapi_encode.c
@@ -412,6 +412,29 @@ static int vaapi_encode_issue(AVCodecContext *avctx,
 }
 }
 
+#if VA_CHECK_VERSION(0, 40, 0)
+if (ctx->vfr_mode && pic->frame_skips > 0) {
+struct {
+VAEncMiscParameterBuffer misc;
+VAEncMiscParameterSkipFrame skip;
+} param = {
+.misc = {
+.type = VAEncMiscParameterTypeSkipFrame,
+},
+.skip = {
+.skip_frame_flag = 1,
+.num_skip_frames = pic->frame_skips,
+},
+};
+
+err = vaapi_encode_make_param_buffer(avctx, pic,
+ VAEncMiscParameterBufferType,
+ (char*)¶m, sizeof(param));
+if (err < 0)
+goto fail;
+}
+#endif
+
 vas = vaBeginPicture(ctx->hwctx->display, ctx->va_context,
  pic->input_surface);
 if (vas != VA_STATUS_SUCCESS) {
@@ -942,6 +965,36 @@ int ff_vaapi_encode_send_frame(AVCodecContext *avctx, 
const AVFrame *frame)
 pic->input_surface = (VASurfaceID)(uintptr_t)frame->data[3];
 pic->pts = frame->pts;
 
+if (ctx->vfr_mode && ctx->input_order > 0) {
+if (frame->pts < ctx->prev_pts) {
+av_log(avctx, AV_LOG_WARNING, "Timestamp discontinuity "
+   "(backward step: %"PRId64" -> %"PRId64"): "
+   "VFR mode reset.\n", ctx->prev_pts, frame->pts);
+ctx->ticks_outstanding = av_make_q(0, 1);
+} else {
+AVRational step_ticks, ticks;
+int ticks_int;
+step_ticks = av_div_q(av_make_q(frame->pts - ctx->prev_pts, 1),
+  ctx->ticks_per_frame);
+ticks = av_add_q(ctx->ticks_outstanding, step_ticks);
+ticks_int = ticks.num / ticks.den;
+if (ticks_int < 1) {
+av_log(avctx, AV_LOG_WARNING, "Max FPS exceeded!\n");
+} else if (ticks_int > 256) {
+av_log(avctx, AV_LOG_WARNING, "Timestamp discontinuity "
+   "(forward step: %"PRId64" -> %"PRId64"): "
+   "VFR mode reset.\n", ctx->prev_pts, frame->pts);
+} else {
+av_log(avctx, AV_LOG_DEBUG, "Inserting %d frame skips 
before "
+   "frame %"PRId64".\n", ticks_int - 1, frame->pts);
+pic->frame_skips = ticks_int - 1;
+}
+ctx->ticks_outstanding =
+av_sub_q(ticks, av_make_q(ticks_int, 1));
+}
+}
+ctx->prev_pts = frame->pts;
+
 if (ctx->input_order == 0)
 ctx->first_pts = pic->pts;
 if (ctx->input_order == ctx->decode_delay)
@@ -1315,7 +1368,6 @@ static av_cold int 
vaapi_encode_init_rate_control(AVCodecContext *avctx)
 int rc_quality;
 int64_t hrd_buffer_size;
 int64_t hrd_initial_buffer_fullness;
-int fr_num, fr_den;
 VAConfigAttrib rc_attr = { VAConfigAttribRateControl };
 VAStatus vas;
 char supported_rc_modes_string[64];
@@ -1609,22 +1661,66 @@ rc_mode_found:
   sizeof(ctx->hrd_params));
 }
 
-if (avctx->framerate.num > 0 && avctx->framerate.den > 0)
-av_reduce(&fr_num, &fr_den,
-  avctx->framerate.num, avctx->framerate.den, 65535);
-else
+return 0;
+}
+
+static av_cold int vaapi_encode_init_framerate(AVCodecContext *avctx)
+{
+VAAPIEncodeContext *ctx = avctx->priv_data;
+
+#if VA_CHECK_VERSION(0, 40, 0)
+int fr_num, fr_den;
+
+ctx-

[FFmpeg-devel] [PATCH v3 08/12] vaapi_encode_mjpeg: Use common quality option

2019-02-10 Thread Mark Thompson
Doesn't change anything, but makes the behaviour better match that of the
other codecs (the CONSTANT_QUALITY_ONLY flag already ensures that CQP is
the only RC mode selectable for MJPEG).
---
 libavcodec/vaapi_encode_mjpeg.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/libavcodec/vaapi_encode_mjpeg.c b/libavcodec/vaapi_encode_mjpeg.c
index f0ea292098..350800697f 100644
--- a/libavcodec/vaapi_encode_mjpeg.c
+++ b/libavcodec/vaapi_encode_mjpeg.c
@@ -438,7 +438,7 @@ static av_cold int 
vaapi_encode_mjpeg_configure(AVCodecContext *avctx)
 VAAPIEncodeMJPEGContext *priv = avctx->priv_data;
 int err;
 
-priv->quality = avctx->global_quality;
+priv->quality = ctx->rc_quality;
 if (priv->quality < 1 || priv->quality > 100) {
 av_log(avctx, AV_LOG_ERROR, "Invalid quality value %d "
"(must be 1-100).\n", priv->quality);
@@ -483,6 +483,8 @@ static const VAAPIEncodeType vaapi_encode_type_mjpeg = {
 
 .configure = &vaapi_encode_mjpeg_configure,
 
+.default_quality   = 80,
+
 .picture_params_size   = sizeof(VAEncPictureParameterBufferJPEG),
 .init_picture_params   = &vaapi_encode_mjpeg_init_picture_params,
 
@@ -536,7 +538,6 @@ static const AVOption vaapi_encode_mjpeg_options[] = {
 };
 
 static const AVCodecDefault vaapi_encode_mjpeg_defaults[] = {
-{ "global_quality", "80" },
 { "b",  "0"  },
 { NULL },
 };
-- 
2.19.2

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


[FFmpeg-devel] [PATCH v3 04/12] vaapi_encode_mpeg2: Enable support for more RC modes

2019-02-10 Thread Mark Thompson
---
 libavcodec/vaapi_encode_mpeg2.c | 24 +---
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/libavcodec/vaapi_encode_mpeg2.c b/libavcodec/vaapi_encode_mpeg2.c
index 9d42c3e644..174611ff24 100644
--- a/libavcodec/vaapi_encode_mpeg2.c
+++ b/libavcodec/vaapi_encode_mpeg2.c
@@ -521,19 +521,17 @@ static av_cold int 
vaapi_encode_mpeg2_configure(AVCodecContext *avctx)
 return err;
 
 if (ctx->va_rc_mode == VA_RC_CQP) {
-priv->quant_p = av_clip(avctx->global_quality, 1, 31);
+priv->quant_p = av_clip(ctx->rc_quality, 1, 31);
 if (avctx->i_quant_factor > 0.0)
-priv->quant_i = av_clip((avctx->global_quality *
- avctx->i_quant_factor +
- avctx->i_quant_offset) + 0.5,
-1, 31);
+priv->quant_i =
+av_clip((avctx->i_quant_factor * priv->quant_p +
+ avctx->i_quant_offset) + 0.5, 1, 31);
 else
 priv->quant_i = priv->quant_p;
 if (avctx->b_quant_factor > 0.0)
-priv->quant_b = av_clip((avctx->global_quality *
- avctx->b_quant_factor +
- avctx->b_quant_offset) + 0.5,
-1, 31);
+priv->quant_b =
+av_clip((avctx->b_quant_factor * priv->quant_p +
+ avctx->b_quant_offset) + 0.5, 1, 31);
 else
 priv->quant_b = priv->quant_p;
 
@@ -542,7 +540,9 @@ static av_cold int 
vaapi_encode_mpeg2_configure(AVCodecContext *avctx)
priv->quant_i, priv->quant_p, priv->quant_b);
 
 } else {
-av_assert0(0 && "Invalid RC mode.");
+priv->quant_i = 16;
+priv->quant_p = 16;
+priv->quant_b = 16;
 }
 
 ctx->slice_block_rows = FFALIGN(avctx->height, 16) / 16;
@@ -567,6 +567,8 @@ static const VAAPIEncodeType vaapi_encode_type_mpeg2 = {
 
 .configure = &vaapi_encode_mpeg2_configure,
 
+.default_quality   = 10,
+
 .sequence_params_size  = sizeof(VAEncSequenceParameterBufferMPEG2),
 .init_sequence_params  = &vaapi_encode_mpeg2_init_sequence_params,
 
@@ -637,6 +639,7 @@ static av_cold int vaapi_encode_mpeg2_close(AVCodecContext 
*avctx)
 #define FLAGS (AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_ENCODING_PARAM)
 static const AVOption vaapi_encode_mpeg2_options[] = {
 VAAPI_ENCODE_COMMON_OPTIONS,
+VAAPI_ENCODE_RC_OPTIONS,
 
 { "profile", "Set profile (in profile_and_level_indication)",
   OFFSET(profile), AV_OPT_TYPE_INT,
@@ -671,7 +674,6 @@ static const AVCodecDefault vaapi_encode_mpeg2_defaults[] = 
{
 { "i_qoffset",  "0"   },
 { "b_qfactor",  "6/5" },
 { "b_qoffset",  "0"   },
-{ "global_quality", "10"  },
 { "qmin",   "-1"  },
 { "qmax",   "-1"  },
 { NULL },
-- 
2.19.2

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


[FFmpeg-devel] [PATCH v3 06/12] vaapi_encode_vp9: Enable support for more RC modes

2019-02-10 Thread Mark Thompson
---
 libavcodec/vaapi_encode_vp9.c | 17 +
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/libavcodec/vaapi_encode_vp9.c b/libavcodec/vaapi_encode_vp9.c
index 97142dcc49..8fb399f115 100644
--- a/libavcodec/vaapi_encode_vp9.c
+++ b/libavcodec/vaapi_encode_vp9.c
@@ -178,23 +178,29 @@ static int 
vaapi_encode_vp9_init_picture_params(AVCodecContext *avctx,
 
 static av_cold int vaapi_encode_vp9_configure(AVCodecContext *avctx)
 {
+VAAPIEncodeContext *ctx = avctx->priv_data;
 VAAPIEncodeVP9Context *priv = avctx->priv_data;
 
-priv->q_idx_p = av_clip(avctx->global_quality, 0, VP9_MAX_QUANT);
+if (ctx->rc_mode->quality) {
+priv->q_idx_p = av_clip(ctx->rc_quality, 0, VP9_MAX_QUANT);
 if (avctx->i_quant_factor > 0.0)
-priv->q_idx_idr = av_clip((avctx->global_quality *
+priv->q_idx_idr = av_clip((priv->q_idx_p *
avctx->i_quant_factor +
avctx->i_quant_offset) + 0.5,
   0, VP9_MAX_QUANT);
 else
 priv->q_idx_idr = priv->q_idx_p;
 if (avctx->b_quant_factor > 0.0)
-priv->q_idx_b = av_clip((avctx->global_quality *
+priv->q_idx_b = av_clip((priv->q_idx_p *
  avctx->b_quant_factor +
  avctx->b_quant_offset) + 0.5,
 0, VP9_MAX_QUANT);
 else
 priv->q_idx_b = priv->q_idx_p;
+} else {
+// Arbitrary value.
+priv->q_idx_idr = priv->q_idx_p = priv->q_idx_b = 100;
+}
 
 return 0;
 }
@@ -211,6 +217,8 @@ static const VAAPIEncodeType vaapi_encode_type_vp9 = {
 .flags = FLAG_B_PICTURES |
  FLAG_B_PICTURE_REFERENCES,
 
+.default_quality   = 100,
+
 .picture_priv_data_size = sizeof(VAAPIEncodeVP9Picture),
 
 .configure = &vaapi_encode_vp9_configure,
@@ -244,6 +252,8 @@ static av_cold int vaapi_encode_vp9_init(AVCodecContext 
*avctx)
 #define FLAGS (AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_ENCODING_PARAM)
 static const AVOption vaapi_encode_vp9_options[] = {
 VAAPI_ENCODE_COMMON_OPTIONS,
+VAAPI_ENCODE_RC_OPTIONS,
+
 { "loop_filter_level", "Loop filter level",
   OFFSET(loop_filter_level), AV_OPT_TYPE_INT, { .i64 = 16 }, 0, 63, FLAGS 
},
 { "loop_filter_sharpness", "Loop filter sharpness",
@@ -255,7 +265,6 @@ static const AVCodecDefault vaapi_encode_vp9_defaults[] = {
 { "b",  "0"   },
 { "bf", "0"   },
 { "g",  "250" },
-{ "global_quality", "100" },
 { "qmin",   "-1"  },
 { "qmax",   "-1"  },
 { NULL },
-- 
2.19.2

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


[FFmpeg-devel] [PATCH v3 12/12] vaapi_encode: Support limiting slice size

2019-02-10 Thread Mark Thompson
---
 doc/encoders.texi |  4 +++
 libavcodec/vaapi_encode.c | 57 ---
 libavcodec/vaapi_encode.h |  9 ++-
 3 files changed, 65 insertions(+), 5 deletions(-)

diff --git a/doc/encoders.texi b/doc/encoders.texi
index 39b8fc1d37..21b6d147fe 100644
--- a/doc/encoders.texi
+++ b/doc/encoders.texi
@@ -2855,6 +2855,10 @@ the stream, and requires driver support.  The minimum 
interval between frames
 must not be smaller than this, and there may be problems if the maximum
 interval is more than a small multiple of it.
 
+@item max_slice_bytes
+Set the maximum number of bytes allowed in a single slice.  Requires driver
+support.  Not limited by default.
+
 @end table
 
 Each encoder also has its own specific options:
diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
index df8f65e431..44d04ca6a9 100644
--- a/libavcodec/vaapi_encode.c
+++ b/libavcodec/vaapi_encode.c
@@ -316,14 +316,14 @@ static int vaapi_encode_issue(AVCodecContext *avctx,
 if (pic->nb_slices == 0)
 pic->nb_slices = ctx->nb_slices;
 if (pic->nb_slices > 0) {
-int rounding;
-
 pic->slices = av_mallocz_array(pic->nb_slices, sizeof(*pic->slices));
 if (!pic->slices) {
 err = AVERROR(ENOMEM);
 goto fail;
 }
-
+}
+if (pic->nb_slices > 0 && ctx->max_slice_bytes == 0) {
+int rounding;
 for (i = 0; i < pic->nb_slices; i++)
 pic->slices[i].row_size = ctx->slice_size;
 
@@ -435,6 +435,31 @@ static int vaapi_encode_issue(AVCodecContext *avctx,
 }
 #endif
 
+#if VA_CHECK_VERSION(1, 0, 0)
+if (ctx->max_slice_bytes > 0) {
+struct {
+VAEncMiscParameterBuffer misc;
+VAEncMiscParameterMaxSliceSize slice;
+} param = {
+.misc = {
+.type = VAEncMiscParameterTypeMaxSliceSize,
+},
+.slice = {
+// VAAPI wants this value in bits.
+.max_slice_size = 8 * ctx->max_slice_bytes,
+},
+};
+
+err = vaapi_encode_make_param_buffer(avctx, pic,
+ VAEncMiscParameterBufferType,
+ (char*)¶m, sizeof(param));
+if (err < 0)
+goto fail;
+}
+#else
+av_assert0(ctx->max_slice_bytes == 0);
+#endif
+
 vas = vaBeginPicture(ctx->hwctx->display, ctx->va_context,
  pic->input_surface);
 if (vas != VA_STATUS_SUCCESS) {
@@ -1812,7 +1837,7 @@ static av_cold int 
vaapi_encode_init_slice_structure(AVCodecContext *avctx)
 ctx->slice_block_cols = (avctx->width  + ctx->slice_block_width  - 1) /
  ctx->slice_block_width;
 
-if (avctx->slices <= 1) {
+if (avctx->slices <= 1 && ctx->max_slice_bytes == 0) {
 ctx->nb_slices  = 1;
 ctx->slice_size = ctx->slice_block_rows;
 return 0;
@@ -1836,6 +1861,30 @@ static av_cold int 
vaapi_encode_init_slice_structure(AVCodecContext *avctx)
 return AVERROR(EINVAL);
 }
 
+if (ctx->max_slice_bytes > 0) {
+#if VA_CHECK_VERSION(1, 0, 0)
+if (slice_structure & VA_ENC_SLICE_STRUCTURE_MAX_SLICE_SIZE) {
+av_log(avctx, AV_LOG_VERBOSE, "Encoding pictures using "
+   "at most %d bytes per slice.\n",
+   ctx->max_slice_bytes);
+// In this mode we supply only a single slice structure and
+// no packed headers - the driver generates the headers for
+// each slice itself.
+ctx->nb_slices = 1;
+ctx->desired_packed_headers &= ~VA_ENC_PACKED_HEADER_SLICE;
+return 0;
+} else {
+av_log(avctx, AV_LOG_ERROR, "Driver does not support "
+   "encoding pictures with a slice size limit.\n");
+return AVERROR(EINVAL);
+}
+#else
+av_log(avctx, AV_LOG_ERROR, "Slice size limiting is "
+   "not supported with this VAAPI version.\n");
+return AVERROR(EINVAL);
+#endif
+}
+
 // For fixed-size slices currently we only support whole rows, making
 // rectangular slices.  This could be extended to arbitrary runs of
 // blocks, but since slices tend to be a conformance requirement and
diff --git a/libavcodec/vaapi_encode.h b/libavcodec/vaapi_encode.h
index 860b67dbe2..ddc947edbe 100644
--- a/libavcodec/vaapi_encode.h
+++ b/libavcodec/vaapi_encode.h
@@ -191,6 +191,9 @@ typedef struct VAAPIEncodeContext {
 // framerate.
 AVRational  vfr_max_fps;
 
+// Requested maximum slice size in bytes.  Ignored if zero.
+unsigned intmax_slice_bytes;
+
 // Desired packed headers.
 unsigned intdesired_packed_headers;
 
@@ -441,7 +444,11 @@ int ff_vaapi_encode_close(AVCodecContext *avctx);
 { "b_depth", \
   "Maximum B-frame reference depth", \
   OFFSET(common.desired_b_depth), AV_OPT_TYPE_INT, \
-  { .i64 = 1 }, 

[FFmpeg-devel] [PATCH v3 07/12] vaapi_encode_vp9: Fix whitespace after previous patch

2019-02-10 Thread Mark Thompson
---
 libavcodec/vaapi_encode_vp9.c | 30 +++---
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/libavcodec/vaapi_encode_vp9.c b/libavcodec/vaapi_encode_vp9.c
index 8fb399f115..f89fd0d07a 100644
--- a/libavcodec/vaapi_encode_vp9.c
+++ b/libavcodec/vaapi_encode_vp9.c
@@ -182,21 +182,21 @@ static av_cold int 
vaapi_encode_vp9_configure(AVCodecContext *avctx)
 VAAPIEncodeVP9Context *priv = avctx->priv_data;
 
 if (ctx->rc_mode->quality) {
-priv->q_idx_p = av_clip(ctx->rc_quality, 0, VP9_MAX_QUANT);
-if (avctx->i_quant_factor > 0.0)
-priv->q_idx_idr = av_clip((priv->q_idx_p *
-   avctx->i_quant_factor +
-   avctx->i_quant_offset) + 0.5,
-  0, VP9_MAX_QUANT);
-else
-priv->q_idx_idr = priv->q_idx_p;
-if (avctx->b_quant_factor > 0.0)
-priv->q_idx_b = av_clip((priv->q_idx_p *
- avctx->b_quant_factor +
- avctx->b_quant_offset) + 0.5,
-0, VP9_MAX_QUANT);
-else
-priv->q_idx_b = priv->q_idx_p;
+priv->q_idx_p = av_clip(ctx->rc_quality, 0, VP9_MAX_QUANT);
+if (avctx->i_quant_factor > 0.0)
+priv->q_idx_idr =
+av_clip((avctx->i_quant_factor * priv->q_idx_p  +
+ avctx->i_quant_offset) + 0.5,
+0, VP9_MAX_QUANT);
+else
+priv->q_idx_idr = priv->q_idx_p;
+if (avctx->b_quant_factor > 0.0)
+priv->q_idx_b =
+av_clip((avctx->b_quant_factor * priv->q_idx_p  +
+ avctx->b_quant_offset) + 0.5,
+0, VP9_MAX_QUANT);
+else
+priv->q_idx_b = priv->q_idx_p;
 } else {
 // Arbitrary value.
 priv->q_idx_idr = priv->q_idx_p = priv->q_idx_b = 100;
-- 
2.19.2

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


[FFmpeg-devel] [PATCH v3 09/12] vaapi_encode_mpeg2: Add missing marker bit in time_code

2019-02-10 Thread Mark Thompson
We don't have anything useful to put in this field, but there is still
meant to be a marker bit in the middle of it.
---
 libavcodec/vaapi_encode_mpeg2.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/libavcodec/vaapi_encode_mpeg2.c b/libavcodec/vaapi_encode_mpeg2.c
index 174611ff24..26d2433e1c 100644
--- a/libavcodec/vaapi_encode_mpeg2.c
+++ b/libavcodec/vaapi_encode_mpeg2.c
@@ -313,7 +313,8 @@ static int 
vaapi_encode_mpeg2_init_sequence_params(AVCodecContext *avctx)
 
 goph->group_start_code = MPEG2_START_GROUP;
 
-goph->time_code   = 0;
+// Marker bit in the middle of time_code.
+goph->time_code   = 1 << 12;
 goph->closed_gop  = 1;
 goph->broken_link = 0;
 
-- 
2.19.2

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


Re: [FFmpeg-devel] [PATCH v2 01/11] vaapi_encode: Support more RC modes

2019-02-10 Thread Mark Thompson
On 05/02/2019 16:51, Eoff, Ullysses A wrote:
>> -Original Message-
>> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf Of 
>> Mark Thompson
>> Sent: Monday, February 04, 2019 1:26 AM
>> To: ffmpeg-devel@ffmpeg.org
>> Subject: Re: [FFmpeg-devel] [PATCH v2 01/11] vaapi_encode: Support more RC 
>> modes
>>
>> On 28/01/2019 04:23, Eoff, Ullysses A wrote:
 -Original Message-
 From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf Of 
 Mark Thompson
 Sent: Sunday, January 27, 2019 3:47 PM
 To: ffmpeg-devel@ffmpeg.org
 Subject: [FFmpeg-devel] [PATCH v2 01/11] vaapi_encode: Support more RC 
 modes

 Allow setting the mode explicitly, and try to make a sensible choice
 given the available parameters if not.
 ---
  doc/encoders.texi |  24 +++
  libavcodec/vaapi_encode.c | 370 +++---
  libavcodec/vaapi_encode.h |  65 +++
  3 files changed, 351 insertions(+), 108 deletions(-)

 ...
  if (rc_attr.value == VA_ATTRIB_NOT_SUPPORTED) {
  av_log(avctx, AV_LOG_VERBOSE, "Driver does not report any "
 -   "supported rate control modes: assuming 
 constant-quality.\n");
 -ctx->va_rc_mode = VA_RC_CQP;
 -return 0;
 ...
>>>
>>> With this patch series, mjpeg_vaapi encoder breaks with iHD driver:
>>>
>>> LIBVA_DRIVER_NAME=iHD ffmpeg -hwaccel vaapi \
>>>   -vaapi_device /dev/dri/renderD128 -v debug \
>>>   -f rawvideo -pix_fmt yuv420p -s:v 1920x1080 \
>>>   -i input.yuv -vf 'format=nv12,hwupload' -c:v mjpeg_vaapi \
>>>   -global_quality 100 -vframes 10 -y output.mjpg
>>>
>>> 
>>> [AVHWDeviceContext @ 0x193c580] Opened VA display via DRM device 
>>> /dev/dri/renderD128.
>>> [AVHWDeviceContext @ 0x193c580] libva: VA-API version 1.4.0
>>> [AVHWDeviceContext @ 0x193c580] libva: va_getDriverName() returns 0
>>> [AVHWDeviceContext @ 0x193c580] libva: User requested driver 'iHD'
>>> [AVHWDeviceContext @ 0x193c580] libva: Trying to open 
>>> /home/uaeoff/Work/workspace/media/install/lib/dri/iHD_drv_video.so
>>> [AVHWDeviceContext @ 0x193c580] libva: Found init function 
>>> __vaDriverInit_1_4
>>> [AVHWDeviceContext @ 0x193c580] libva: va_openDriver() returns 0
>>> [AVHWDeviceContext @ 0x193c580] Initialised VAAPI connection: version 1.4
>>> 
>>> [mjpeg_vaapi @ 0x19847c0] Input surface format is nv12.
>>> [mjpeg_vaapi @ 0x19847c0] Using VAAPI profile VAProfileJPEGBaseline (12).
>>> [mjpeg_vaapi @ 0x19847c0] Using VAAPI entrypoint VAEntrypointEncPicture (7).
>>> [mjpeg_vaapi @ 0x19847c0] Using VAAPI render target format YUV420 (0x1).
>>> [mjpeg_vaapi @ 0x19847c0] Driver does not report any supported rate control 
>>> modes: assuming CQP only.
>>> [mjpeg_vaapi @ 0x19847c0] RC mode: CQP.
>>> [mjpeg_vaapi @ 0x19847c0] RC quality: 100.
>>> [mjpeg_vaapi @ 0x19847c0] RC framerate (CFR mode): 25/1 (25.00 fps).
>>> [mjpeg_vaapi @ 0x19847c0] Using intra frames only.
>>> [mjpeg_vaapi @ 0x19847c0] All wanted packed headers available (wanted 0x10, 
>>> found 0x10).
>>> [mjpeg_vaapi @ 0x19847c0] Failed to create encode pipeline configuration: 
>>> 10 (attribute not supported).
>>> Error initializing output stream 0:0 -- Error while opening encoder for 
>>> output stream #0:0 - maybe incorrect parameters such as
>> bit_rate, rate, width or height
>>> [AVIOContext @ 0x1987000] Statistics: 0 seeks, 0 writeouts
>>> [AVIOContext @ 0x19802c0] Statistics: 3110400 bytes read, 0 seeks
>>> Conversion failed!
>>>
>>> ... it works fine on i965 driver.
>>
>> Right, the specific workaround to not set any options on drivers which 
>> report no supported RC modes (highlighted above) needs to be
>> carried forward to this.  Added.
>>
>> Thank you for testing!
>>
> 
> Is there a v3 I can test?  Thanks.

New version sent.  It now doesn't bail out from that function early because it 
does want to do some other setup there; instead it just avoids setting that 
particular attribute at the end if the driver has declared that it doesn't 
support it.

Thanks,

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


[FFmpeg-devel] [PATCH] avcodec/cbs_av1: change the assert in trailing_bits to accept zero bits when reading

2019-02-10 Thread James Almer
If nb_bits is zero when reading an OBU, then it's not a bug in CBS but an
invalid bitstream, and we should abort gracefully instead.

Signed-off-by: James Almer 
---
rav1e is currently encoding invalid Metadata OBUs without trailing bits, which
are triggering the assert when parsed by CBS. This change makes sure to instead
report the bitstream as invalid and gracefully return with an error code
instead of crashing.

 libavcodec/cbs_av1_syntax_template.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/libavcodec/cbs_av1_syntax_template.c 
b/libavcodec/cbs_av1_syntax_template.c
index 48f4fab514..02b4ed221c 100644
--- a/libavcodec/cbs_av1_syntax_template.c
+++ b/libavcodec/cbs_av1_syntax_template.c
@@ -45,7 +45,11 @@ static int FUNC(trailing_bits)(CodedBitstreamContext *ctx, 
RWContext *rw, int nb
 {
 int err;
 
+#ifdef READ
+av_assert0(nb_bits >= 0);
+#else
 av_assert0(nb_bits > 0);
+#endif
 
 fixed(1, trailing_one_bit, 1);
 --nb_bits;
-- 
2.20.1

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


Re: [FFmpeg-devel] [PATCH] avcodec/cbs_av1: change the assert in trailing_bits to accept zero bits when reading

2019-02-10 Thread James Almer
On 2/10/2019 5:12 PM, James Almer wrote:
> If nb_bits is zero when reading an OBU, then it's not a bug in CBS but an
> invalid bitstream, and we should abort gracefully instead.
> 
> Signed-off-by: James Almer 
> ---
> rav1e is currently encoding invalid Metadata OBUs without trailing bits, which
> are triggering the assert when parsed by CBS. This change makes sure to 
> instead
> report the bitstream as invalid and gracefully return with an error code
> instead of crashing.
> 
>  libavcodec/cbs_av1_syntax_template.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/libavcodec/cbs_av1_syntax_template.c 
> b/libavcodec/cbs_av1_syntax_template.c
> index 48f4fab514..02b4ed221c 100644
> --- a/libavcodec/cbs_av1_syntax_template.c
> +++ b/libavcodec/cbs_av1_syntax_template.c
> @@ -45,7 +45,11 @@ static int FUNC(trailing_bits)(CodedBitstreamContext *ctx, 
> RWContext *rw, int nb
>  {
>  int err;
>  
> +#ifdef READ
> +av_assert0(nb_bits >= 0);
> +#else
>  av_assert0(nb_bits > 0);
> +#endif
>  
>  fixed(1, trailing_one_bit, 1);
>  --nb_bits;

Better version approved by Mark on IRC pushed instead.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avformat/mov: fix hang while seek on a kind of fragmented mp4.

2019-02-10 Thread Marton Balint



On Sun, 3 Feb 2019, Charles Liu wrote:


Binary searching would hang if the fragment items do NOT have timestamp for the 
specified stream.

For example, a fmp4 consists of separated 'moof' boxes for each track, and 
separated 'sidx' for each segment, but no 'mfra' box.
Then every fragment item only have the timestamp for one of its tracks.

Signed-off-by: Charles Liu 
---
libavformat/mov.c | 21 -
1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/libavformat/mov.c b/libavformat/mov.c
index 9b9739f788..35cb619e9f 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -1266,7 +1266,7 @@ static int64_t get_frag_time(MOVFragmentIndex *frag_index,
static int search_frag_timestamp(MOVFragmentIndex *frag_index,
 AVStream *st, int64_t timestamp)
{
-int a, b, m;
+int a, b, m, m0;
int64_t frag_time;
int id = -1;

@@ -1282,15 +1282,18 @@ static int search_frag_timestamp(MOVFragmentIndex 
*frag_index,
b = frag_index->nb_items;

while (b - a > 1) {
-m = (a + b) >> 1;
-frag_time = get_frag_time(frag_index, m, id);
-if (frag_time != AV_NOPTS_VALUE) {
-if (frag_time >= timestamp)
-b = m;
-if (frag_time <= timestamp)
-a = m;
-}
+m0 = m = (a + b) >> 1;
+
+while (m < b &&
+   (frag_time = get_frag_time(frag_index, m, id)) == 
AV_NOPTS_VALUE)
+m++;
+
+if (m < b && frag_time <= timestamp)
+a = m;
+else
+b = m0;
}
+
return a;
}



As this fixes a hang, I will push this version soon. Can be optimized 
later to pure binary search, if anybody is still interested.


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


Re: [FFmpeg-devel] [PATCH] configure: warn about disabled explicitly enabled components

2019-02-10 Thread Marton Balint



On Sat, 9 Feb 2019, Marton Balint wrote:




On Sat, 9 Feb 2019, Carl Eugen Hoyos wrote:


2019-02-05 22:14 GMT+01:00, Marton Balint :

If we enable a component but a dependant library is disabled, then the
enabled
component gets silently disabled. Warning about disabled explicitly 

enabled

components
allows configure to show the missing dependencies and if --fatal-warnings 

is

used it can also fail if the user wants it so.

For example if libdav1d is not availble ./configure
--enable-decoder=libdav1d
succeeds but the libdav1d decoder is not be enabled. After the patch
configure
will warn about this:

WARNING: Disabled libdav1d_decoder because not all dependencies are
satisfied: libdav1d


The patch produces many warnings for:
$ ./configure --disable-everything --enable-decoder=mp*
Is this intended?


It reports only disabled decoders which start with mp, so it works as it 
should the way I see it.




Will push this soon.

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


Re: [FFmpeg-devel] [PATCH] avformat/mov: fix hang while seek on a kind of fragmented mp4.

2019-02-10 Thread Carl Eugen Hoyos
2019-02-10 23:04 GMT+01:00, Marton Balint :
>
>
> On Sun, 3 Feb 2019, Charles Liu wrote:
>
>> Binary searching would hang if the fragment items do NOT have timestamp
>> for the specified stream.
>>
>> For example, a fmp4 consists of separated 'moof' boxes for each track, and
>> separated 'sidx' for each segment, but no 'mfra' box.
>> Then every fragment item only have the timestamp for one of its tracks.
>>
>> Signed-off-by: Charles Liu 
>> ---
>> libavformat/mov.c | 21 -
>> 1 file changed, 12 insertions(+), 9 deletions(-)
>>
>> diff --git a/libavformat/mov.c b/libavformat/mov.c
>> index 9b9739f788..35cb619e9f 100644
>> --- a/libavformat/mov.c
>> +++ b/libavformat/mov.c
>> @@ -1266,7 +1266,7 @@ static int64_t get_frag_time(MOVFragmentIndex
>> *frag_index,
>> static int search_frag_timestamp(MOVFragmentIndex *frag_index,
>>  AVStream *st, int64_t timestamp)
>> {
>> -int a, b, m;
>> +int a, b, m, m0;
>> int64_t frag_time;
>> int id = -1;
>>
>> @@ -1282,15 +1282,18 @@ static int search_frag_timestamp(MOVFragmentIndex
>> *frag_index,
>> b = frag_index->nb_items;
>>
>> while (b - a > 1) {
>> -m = (a + b) >> 1;
>> -frag_time = get_frag_time(frag_index, m, id);
>> -if (frag_time != AV_NOPTS_VALUE) {
>> -if (frag_time >= timestamp)
>> -b = m;
>> -if (frag_time <= timestamp)
>> -a = m;
>> -}
>> +m0 = m = (a + b) >> 1;
>> +
>> +while (m < b &&
>> +   (frag_time = get_frag_time(frag_index, m, id)) ==
>> AV_NOPTS_VALUE)
>> +m++;
>> +
>> +if (m < b && frag_time <= timestamp)
>> +a = m;
>> +else
>> +b = m0;
>> }
>> +
>> return a;
>> }
>>
>
> As this fixes a hang, I will push this version soon.

Please mention ticket #7572 in the commit message.

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


Re: [FFmpeg-devel] [PATCH 1/4] zmbvenc: don't sum the entropy when blocks are equal

2019-02-10 Thread Matthew Fearnley
On Thu, 31 Jan 2019 at 15:00, Tomas Härdin  wrote:

>
> > 1. The entropy calculation in block_cmp() omits the score of histogram[0]
> > from the final sum.
> > It's tempting to do this to bias the scores in favour of 0-bytes, but in
> > reality, blocks with a majority of 0 (or any other byte) will already be
> > naturally favoured by the entropy score anyway, and this bias will fail
> to
> > penalise blocks with an "average" (i.e. high entropy) number of 0's in
> them.
> > The exact implications are difficult to ponder, but in practical terms
> this
> > error does tend to produce worse results in the video compression.  Not
> > massively so, but it's still noticeable.
>
> Did you try combining the statistics of the current MV with the
> statistics of the previous block, to bias the choice in favor of
> similar bytes? Could work like a cheap order-1 entropy model.
>

I've had a go at this, but sadly, it seemed to adversely affect
compression, producing larger files.
Maybe it can be improved with some tweaking, or maybe there's a bug
somewhere.
But it feels to me like this approach on its own isn't enough to improve
scoring.  Maybe as part of something larger though..
Anyway, you can see the results of my efforts here:
https://github.com/countingpine/FFmpeg/commit/prev_histogram.

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


Re: [FFmpeg-devel] [PATCH 1/3] libavcodec/cbs: Add ability to keep the units array.

2019-02-10 Thread Mark Thompson
On 05/02/2019 20:08, Andreas Rheinhardt wrote:
> Currently, a fragment's unit array is constantly reallocated during
> splitting of a packet. This commit adds the ability to keep the unit
> array by distinguishing between the number of allocated and the number
> of valid units in the unit array.
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/av1_metadata_bsf.c   |  4 +--
>  libavcodec/av1_parser.c |  4 +--
>  libavcodec/cbs.c| 51 ++---
>  libavcodec/cbs.h| 19 ---
>  libavcodec/filter_units_bsf.c   |  6 ++--
>  libavcodec/h264_metadata_bsf.c  |  4 +--
>  libavcodec/h264_redundant_pps_bsf.c |  4 +--
>  libavcodec/h265_metadata_bsf.c  |  4 +--
>  libavcodec/mpeg2_metadata_bsf.c |  4 +--
>  libavcodec/trace_headers_bsf.c  |  4 +--
>  libavcodec/vaapi_encode_h264.c  |  8 ++---
>  libavcodec/vaapi_encode_h265.c  |  8 ++---
>  libavcodec/vaapi_encode_mjpeg.c |  2 +-
>  libavcodec/vaapi_encode_mpeg2.c |  4 +--
>  libavcodec/vp9_metadata_bsf.c   |  2 +-
>  15 files changed, 76 insertions(+), 52 deletions(-)
> 
> ...
> diff --git a/libavcodec/cbs.c b/libavcodec/cbs.c
> index ecbf57c293..b61dedb1eb 100644
> --- a/libavcodec/cbs.c
> +++ b/libavcodec/cbs.c
> @@ -137,15 +137,20 @@ static void cbs_unit_uninit(CodedBitstreamContext *ctx,
>  }
>  
>  void ff_cbs_fragment_uninit(CodedBitstreamContext *ctx,
> -CodedBitstreamFragment *frag)
> +CodedBitstreamFragment *frag,
> +int completely)
>  {
>  int i;
>  
>  for (i = 0; i < frag->nb_units; i++)
>  cbs_unit_uninit(ctx, &frag->units[i]);
> -av_freep(&frag->units);
>  frag->nb_units = 0;
>  
> +if (completely) {
> +av_freep(&frag->units);
> +frag->units_allocated = 0;
> +}
> +
>  av_buffer_unref(&frag->data_ref);
>  frag->data = NULL;
>  frag->data_size= 0;
> @@ -548,20 +553,34 @@ static int cbs_insert_unit(CodedBitstreamContext *ctx,
>  {
>  CodedBitstreamUnit *units;
>  
> -units = av_malloc_array(frag->nb_units + 1, sizeof(*units));
> -if (!units)
> -return AVERROR(ENOMEM);
> +if (frag->nb_units < frag->units_allocated) {
> +units = frag->units;
> +
> +if (position < frag->nb_units)
> +memmove(units + position + 1, frag->units + position,
> +(frag->nb_units - position) * sizeof(*units));
> +} else {
> +units = av_malloc_array(frag->nb_units + 1, sizeof(*units));
> +if (!units)
> +return AVERROR(ENOMEM);
> +
> +++frag->units_allocated;

Given your aim, I wonder whether it would be sensible to have this increase the 
size by more the one each time - maybe double it or add a small constant (four 
or eight)?  The unit structures themselves are not particularly large, so this 
might save a decent number of allocations at the cost of wasting perhaps few 
hundred bytes of memory.

>  
> -if (position > 0)
> -memcpy(units, frag->units, position * sizeof(*units));
> -if (position < frag->nb_units)
> -memcpy(units + position + 1, frag->units + position,
> -   (frag->nb_units - position) * sizeof(*units));
> +if (position > 0)
> +memcpy(units, frag->units, position * sizeof(*units));
> +
> +if (position < frag->nb_units)
> +memcpy(units + position + 1, frag->units + position,
> +   (frag->nb_units - position) * sizeof(*units));
> +}
>  
>  memset(units + position, 0, sizeof(*units));
>  
> -av_freep(&frag->units);
> -frag->units = units;
> +if (units != frag->units) {
> +av_free(frag->units);
> +frag->units = units;
> +}
> +
>  ++frag->nb_units;
>  
>  return 0;
> @@ -652,16 +671,10 @@ int ff_cbs_delete_unit(CodedBitstreamContext *ctx,
>  
>  --frag->nb_units;
>  
> -if (frag->nb_units == 0) {
> -av_freep(&frag->units);
> -
> -} else {
> +if (frag->nb_units > 0)
>  memmove(frag->units + position,
>  frag->units + position + 1,
>  (frag->nb_units - position) * sizeof(*frag->units));
>  
> -// Don't bother reallocating the unit array.
> -}
> -
>  return 0;
>  }
> diff --git a/libavcodec/cbs.h b/libavcodec/cbs.h
> index 53ac360bb1..229cb129aa 100644
> --- a/libavcodec/cbs.h
> +++ b/libavcodec/cbs.h
> @@ -145,10 +145,19 @@ typedef struct CodedBitstreamFragment {
>   * and has not been decomposed.
>   */
>  int  nb_units;
> +
> +/**
> + * Number of allocated units.
> + *
> + * Must always be >= nb_units; designed for internal use by cbs.
> + */
> + int  units_allocated;

I think call this nb_units_allocated for consistent styling.

> +
>  /**
> - * Pointer to an array of units of length nb_units.
> +   

Re: [FFmpeg-devel] [PATCH 2/3] libavcodec/cbs: Don't zero twice

2019-02-10 Thread Mark Thompson
On 05/02/2019 20:08, Andreas Rheinhardt wrote:
> Up until now, a fragment that got reused was zeroed twice: Once during
> uninit and once during reading the next packet/extradata/buffer. The
> second zeroing has now been made optional.
> 
> This is also in preparation of actually reusing a fragment's units array.
> Otherwise it would be lost during reading.
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/av1_metadata_bsf.c   |  4 ++--
>  libavcodec/av1_parser.c |  4 ++--
>  libavcodec/cbs.c| 17 +++--
>  libavcodec/cbs.h| 20 +---
>  libavcodec/filter_units_bsf.c   |  4 ++--
>  libavcodec/h264_metadata_bsf.c  |  4 ++--
>  libavcodec/h264_redundant_pps_bsf.c |  4 ++--
>  libavcodec/h265_metadata_bsf.c  |  4 ++--
>  libavcodec/mpeg2_metadata_bsf.c |  4 ++--
>  libavcodec/trace_headers_bsf.c  |  4 ++--
>  libavcodec/vp9_metadata_bsf.c   |  2 +-
>  11 files changed, 45 insertions(+), 26 deletions(-)
> 
> ...
> diff --git a/libavcodec/cbs.c b/libavcodec/cbs.c
> index b61dedb1eb..71f9fcbe32 100644
> --- a/libavcodec/cbs.c
> +++ b/libavcodec/cbs.c
> @@ -217,11 +217,13 @@ static int cbs_fill_fragment_data(CodedBitstreamContext 
> *ctx,
>  
>  int ff_cbs_read_extradata(CodedBitstreamContext *ctx,
>CodedBitstreamFragment *frag,
> -  const AVCodecParameters *par)
> +  const AVCodecParameters *par,
> +  int reuse)
>  {
>  int err;
>  
> -memset(frag, 0, sizeof(*frag));
> +if (!reuse)
> +memset(frag, 0, sizeof(*frag));
>  
>  err = cbs_fill_fragment_data(ctx, frag, par->extradata,
>   par->extradata_size);
> @@ -237,11 +239,12 @@ int ff_cbs_read_extradata(CodedBitstreamContext *ctx,
>  
>  int ff_cbs_read_packet(CodedBitstreamContext *ctx,
> CodedBitstreamFragment *frag,
> -   const AVPacket *pkt)
> +   const AVPacket *pkt, int reuse)
>  {
>  int err;
>  
> -memset(frag, 0, sizeof(*frag));
> +if (!reuse)
> +memset(frag, 0, sizeof(*frag));
>  
>  if (pkt->buf) {
>  frag->data_ref = av_buffer_ref(pkt->buf);
> @@ -266,11 +269,13 @@ int ff_cbs_read_packet(CodedBitstreamContext *ctx,
>  
>  int ff_cbs_read(CodedBitstreamContext *ctx,
>  CodedBitstreamFragment *frag,
> -const uint8_t *data, size_t size)
> +const uint8_t *data, size_t size,
> +int reuse)
>  {
>  int err;
>  
> -memset(frag, 0, sizeof(*frag));
> +if (!reuse)
> +memset(frag, 0, sizeof(*frag));
>  
>  err = cbs_fill_fragment_data(ctx, frag, data, size);
>  if (err < 0)
> diff --git a/libavcodec/cbs.h b/libavcodec/cbs.h
> index 229cb129aa..2265b5d5bd 100644
> --- a/libavcodec/cbs.h
> +++ b/libavcodec/cbs.h
> @@ -240,10 +240,15 @@ void ff_cbs_close(CodedBitstreamContext **ctx);
>   * This also updates the internal state, so will need to be called for
>   * codecs with extradata to read parameter sets necessary for further
>   * parsing even if the fragment itself is not desired.
> + *
> + * If reuse is not set, the fragment will be zeroed before usage;
> + * otherwise, the fragment's units_allocated and units members must
> + * be valid and all the other members have to be zero/NULL.
>   */
>  int ff_cbs_read_extradata(CodedBitstreamContext *ctx,
>CodedBitstreamFragment *frag,
> -  const AVCodecParameters *par);
> +  const AVCodecParameters *par,
> +  int reuse);
>  
>  /**
>   * Read the data bitstream from a packet into a fragment, then
> @@ -252,10 +257,14 @@ int ff_cbs_read_extradata(CodedBitstreamContext *ctx,
>   * This also updates the internal state of the coded bitstream context
>   * with any persistent data from the fragment which may be required to
>   * read following fragments (e.g. parameter sets).
> + *
> + * If reuse is not set, the fragment will be zeroed before usage;
> + * otherwise, the fragment's units_allocated and units members must
> + * be valid and all the other members have to be zero/NULL.
>   */
>  int ff_cbs_read_packet(CodedBitstreamContext *ctx,
> CodedBitstreamFragment *frag,
> -   const AVPacket *pkt);
> +   const AVPacket *pkt, int reuse);
>  
>  /**
>   * Read a bitstream from a memory region into a fragment, then
> @@ -264,10 +273,15 @@ int ff_cbs_read_packet(CodedBitstreamContext *ctx,
>   * This also updates the internal state of the coded bitstream context
>   * with any persistent data from the fragment which may be required to
>   * read following fragments (e.g. parameter sets).
> + *
> + * If reuse is not set, the fragment will be zeroed before usage;
> + * otherwise, the fragment's units_allocated and uni

Re: [FFmpeg-devel] [PATCH 1/2 v3] add libaribb24 ARIB STD-B24 caption decoder

2019-02-10 Thread Michael Niedermayer
On Sun, Feb 03, 2019 at 09:38:40PM +0200, Jan Ekström wrote:
> * Outputs ASS lines with basic coloring and font scaling for each
>   given region.
> * Sets the default style to the resolution of the subtitle plane
>   (for example, 960x540 / 36pt font for profile A).
> * Has options to:
>   * Disable ruby text (which is coded as regions which have
> half-height text in libaribb24).
> Enabled by default as without positioning ruby text only
> confuses as it is usually coded in the beginning of the decoded
> subtitle line.
>   * Set the working directory, in which libaribb24 will read
> configuration as well as into which it may save broadcast extra
> symbols as PNG.
> Unset by default.
> 
> The unconventional library check can be explained by the library's
> current master branch being licensed as LGPLv3, but at the time of
> writing the latest official release is still licensed under GPLv3.
> 
> Thus, one either has to wait for the following release, or enable
> GPLv3.
> ---
> 
> Version 3 changes:
>   - Pre-converted subtitle region colors to BGR in local variables
> to make checks work even in cases where the default colors are not
> all 0xFF or 0x00-filled, and to use BGR values consistently.
>   - Added extra parenthesis around the active parts of the RGB_TO_BGR
> macro to separate its content from the position it is used in.
> 
> Version 2 changes:
>   - Corrected colors by adding a macro that converts from RGB to BGR,
> I had forgotten that ASS is not RGB.
> Thanks to a nice person from the .jp DTV community to have
> noticed this.
> 
>  Changelog   |   1 +
>  configure   |   7 +
>  doc/decoders.texi   |  25 +++
>  libavcodec/Makefile |   1 +
>  libavcodec/allcodecs.c  |   1 +
>  libavcodec/avcodec.h|   4 +
>  libavcodec/codec_desc.c |   9 +-
>  libavcodec/libaribb24.c | 384 
>  libavcodec/profiles.c   |   6 +
>  libavcodec/profiles.h   |   1 +
>  libavcodec/version.h|   2 +-
>  11 files changed, 439 insertions(+), 2 deletions(-)
>  create mode 100644 libavcodec/libaribb24.c
> 
> diff --git a/Changelog b/Changelog
> index 06cf6bf080..d5515c4911 100644
> --- a/Changelog
> +++ b/Changelog
> @@ -17,6 +17,7 @@ version :
>  - maskfun filter
>  - hcom demuxer and decoder
>  - ARBC decoder
> +- libaribb24 based ARIB STD-B24 caption support (profiles A and C)
>  
>  
>  version 4.1:
> diff --git a/configure b/configure
> index e1412352fa..cb852c6289 100755
> --- a/configure
> +++ b/configure
> @@ -218,6 +218,7 @@ External library support:
>--enable-jni enable JNI support [no]
>--enable-ladspa  enable LADSPA audio filtering [no]
>--enable-libaom  enable AV1 video encoding/decoding via libaom [no]
> +  --enable-libaribb24  enable ARIB text and caption decoding via 
> libaribb24 [no]
>--enable-libass  enable libass subtitles rendering,
> needed for subtitles and ass filter [no]
>--enable-libbluray   enable BluRay reading using libbluray [no]
> @@ -1684,6 +1685,7 @@ EXTERNAL_LIBRARY_NONFREE_LIST="
>  EXTERNAL_LIBRARY_VERSION3_LIST="
>  gmp
>  liblensfun
> +libaribb24
>  libopencore_amrnb
>  libopencore_amrwb
>  libvmaf
> @@ -1707,6 +1709,7 @@ EXTERNAL_LIBRARY_LIST="
>  jni
>  ladspa
>  libaom
> +libaribb24
>  libass
>  libbluray
>  libbs2b
> @@ -3094,6 +3097,7 @@ hevc_videotoolbox_encoder_select="videotoolbox_encoder"
>  libaom_av1_decoder_deps="libaom"
>  libaom_av1_encoder_deps="libaom"
>  libaom_av1_encoder_select="extract_extradata_bsf"
> +libaribb24_decoder_deps="libaribb24"
>  libcelt_decoder_deps="libcelt"
>  libcodec2_decoder_deps="libcodec2"
>  libcodec2_encoder_deps="libcodec2"
> @@ -6080,6 +6084,9 @@ enabled gnutls&& require_pkg_config gnutls 
> gnutls gnutls/gnutls.h gn
>  enabled jni   && { [ $target_os = "android" ] && check_headers 
> jni.h && enabled pthreads || die "ERROR: jni not found"; }
>  enabled ladspa&& require_headers "ladspa.h dlfcn.h"
>  enabled libaom&& require_pkg_config libaom "aom >= 1.0.0" 
> aom/aom_codec.h aom_codec_version
> +enabled libaribb24&& { check_pkg_config libaribb24 "aribb24 > 1.0.3" 
> "aribb24/aribb24.h" arib_instance_new ||
> +   { enabled gpl && require_pkg_config 
> libaribb24 aribb24 "aribb24/aribb24.h" arib_instance_new; } ||
> +   die "ERROR: libaribb24 requires version 
> higher than 1.0.3 or --enable-gpl."; }
>  enabled lv2   && require_pkg_config lv2 lilv-0 "lilv/lilv.h" 
> lilv_world_new
>  enabled libiec61883   && require libiec61883 libiec61883/iec61883.h 
> iec61883_cmp_connect -lraw1394 -lavc1394 -lrom1394 -liec61883
>  enabled libass&& require_pkg_config libass libass ass/ass.h 
> ass_library_init
> diff --git a/doc/decoders.texi b/doc/decoder

Re: [FFmpeg-devel] [PATCH 3/3] cbs: Stop reallocating fragment unit arrays

2019-02-10 Thread Mark Thompson
On 05/02/2019 20:08, Andreas Rheinhardt wrote:
> This commit changes various places that make use of cbs to keep the
> fragments' unit arrays instead of constantly reallocating them.
> 
> The more units a packet is split into, the bigger the benefit.
> So MPEG-2 benefits the most; for a video coming from an NTSC-DVD
> (usually 32 units per frame) the average cost of cbs_insert_unit (for a
> single unit) went down from 6717 decicycles to 450 decicycles (based
> upon 10 runs with 4194304 runs each); if each packet consists of only
> one unit, it went down from 2425 to 448; for a H.264 video where most
> packets contain nine units, it went from 4431 to 450.
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/av1_metadata_bsf.c   |  6 --
>  libavcodec/av1_parser.c |  5 +++--
>  libavcodec/filter_units_bsf.c   | 13 +++--
>  libavcodec/h264_metadata_bsf.c  |  6 --
>  libavcodec/h264_redundant_pps_bsf.c |  6 --
>  libavcodec/h265_metadata_bsf.c  |  6 --
>  libavcodec/mpeg2_metadata_bsf.c |  6 --
>  libavcodec/trace_headers_bsf.c  | 14 --
>  libavcodec/vp9_metadata_bsf.c   |  4 +++-
>  9 files changed, 41 insertions(+), 25 deletions(-)
> 
> diff --git a/libavcodec/av1_metadata_bsf.c b/libavcodec/av1_metadata_bsf.c
> index b08a1379e7..5e4dc10b89 100644
> --- a/libavcodec/av1_metadata_bsf.c
> +++ b/libavcodec/av1_metadata_bsf.c
> @@ -170,7 +170,7 @@ static int av1_metadata_filter(AVBSFContext *bsf, 
> AVPacket *out)
>  
>  err = 0;
>  fail:
> -ff_cbs_fragment_uninit(ctx->cbc, frag, 1);
> +ff_cbs_fragment_uninit(ctx->cbc, frag, 0);
>  
>  if (err < 0)
>  av_packet_unref(out);
> @@ -215,13 +215,15 @@ static int av1_metadata_init(AVBSFContext *bsf)
>  
>  err = 0;
>  fail:
> -ff_cbs_fragment_uninit(ctx->cbc, frag, 1);
> +ff_cbs_fragment_uninit(ctx->cbc, frag, 0);
>  return err;
>  }
>  
>  static void av1_metadata_close(AVBSFContext *bsf)
>  {
>  AV1MetadataContext *ctx = bsf->priv_data;
> +
> +ff_cbs_fragment_uninit(ctx->cbc, &ctx->access_unit, 1);
>  ff_cbs_close(&ctx->cbc);
>  }

I feel like this patch for each BSF demonstrates nicely that they should be 
separate functions - one version is called repeatedly in the filter operations, 
the other only in close.

With suitable names, this all looks good.

Thanks,

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


Re: [FFmpeg-devel] [PATCH 2/3] libavcodec/cbs: Don't zero twice

2019-02-10 Thread Andreas Rheinhardt
Mark Thompson:
> On 05/02/2019 20:08, Andreas Rheinhardt wrote:
>>  int ff_cbs_read_extradata(CodedBitstreamContext *ctx,
>>CodedBitstreamFragment *frag,
>> -  const AVCodecParameters *par)
>> +  const AVCodecParameters *par,
>> +  int reuse)
>>  {
>>  int err;
>>  
>> -memset(frag, 0, sizeof(*frag));
>> +if (!reuse)
>> +memset(frag, 0, sizeof(*frag));
>>  
>>  err = cbs_fill_fragment_data(ctx, frag, par->extradata,
>>   par->extradata_size);
>> @@ -237,11 +239,12 @@ int ff_cbs_read_extradata(CodedBitstreamContext *ctx,
>>  
>>  int ff_cbs_read_packet(CodedBitstreamContext *ctx,
>> CodedBitstreamFragment *frag,
>> -   const AVPacket *pkt)
>> +   const AVPacket *pkt, int reuse)
>>  {
>>  int err;
>>  
>> -memset(frag, 0, sizeof(*frag));
>> +if (!reuse)
>> +memset(frag, 0, sizeof(*frag));
>>  
>>  if (pkt->buf) {
>>  frag->data_ref = av_buffer_ref(pkt->buf);
>> @@ -266,11 +269,13 @@ int ff_cbs_read_packet(CodedBitstreamContext *ctx,
>>  
>>  int ff_cbs_read(CodedBitstreamContext *ctx,
>>  CodedBitstreamFragment *frag,
>> -const uint8_t *data, size_t size)
>> +const uint8_t *data, size_t size,
>> +int reuse)
>>  {
>>  int err;
>>  
>> -memset(frag, 0, sizeof(*frag));
>> +if (!reuse)
>> +memset(frag, 0, sizeof(*frag));
>>  
>>  err = cbs_fill_fragment_data(ctx, frag, data, size);
>>  if (err < 0)
> 
> I don't think this patch should be needed.  Can we just document that your 
> fragment must either be zeroed (so, allocated by av_*allocz() or memset() to 
> zero) or you've called the ff_cbs_fragment_reset() (whatever the name is) 
> function before any read call?  It can even check that the user hasn't messed 
> up by asserting that data, data_size and nb_units are all zero.

I agree with your suggestion regarding the documentation; but it is
nevertheless needed to eliminate the memset in the ff_cbs_read
functions. Otherwise the unit array is leaked.

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


Re: [FFmpeg-devel] [PATCH 2/2] tests/api/api-h264-test: Add AV_NOPTS_VALUE check for AVFrame.pkt_dts

2019-02-10 Thread Michael Niedermayer
On Sun, Feb 10, 2019 at 02:53:58PM +0800, Jun Zhao wrote:
> Add AV_NOPTS_VALUE check for AVFrame.pkt_dts to avoid print the
> pkt_dts as negative number like:
> "0,3616613, -9223372036854775808, 1001,  3110400, 0x75e37a65"
> 
> Signed-off-by: Jun Zhao 
> ---
>  tests/api/api-h264-test.c |   12 +---
>  1 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/api/api-h264-test.c b/tests/api/api-h264-test.c
> index 9fa..351d622 100644
> --- a/tests/api/api-h264-test.c
> +++ b/tests/api/api-h264-test.c
> @@ -131,9 +131,15 @@ static int video_decode_example(const char 
> *input_filename)
>  av_log(NULL, AV_LOG_ERROR, "Can't copy image to 
> buffer\n");
>  return number_of_written_bytes;
>  }
> -printf("%d, %10"PRId64", %10"PRId64", %8"PRId64", %8d, 
> 0x%08lx\n", video_stream,
> -fr->pts, fr->pkt_dts, fr->pkt_duration,
> -number_of_written_bytes, av_adler32_update(0, (const 
> uint8_t*)byte_buffer, number_of_written_bytes));
> +
> +if (fr->pkt_dts == AV_NOPTS_VALUE)
> +printf("%d, %10"PRId64", %s, %8"PRId64", %8d, 
> 0x%08lx\n", video_stream,
> +   fr->pts, "N/A", fr->pkt_duration,
> +   number_of_written_bytes, av_adler32_update(0, 
> (const uint8_t*)byte_buffer, number_of_written_bytes));
> +else
> +printf("%d, %10"PRId64", %10"PRId64", %8"PRId64", %8d, 
> 0x%08lx\n", video_stream,
> +   fr->pts, fr->pkt_dts, fr->pkt_duration,
> +   number_of_written_bytes, av_adler32_update(0, 
> (const uint8_t*)byte_buffer, number_of_written_bytes));

you can split the printf() in 2 so the common parts are not duplicated

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The smallest minority on earth is the individual. Those who deny 
individual rights cannot claim to be defenders of minorities. - Ayn Rand


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 v3] add libaribb24 ARIB STD-B24 caption decoder

2019-02-10 Thread Jan Ekström
On Mon, Feb 11, 2019 at 1:01 AM Michael Niedermayer  wrote:
>
> On Sun, Feb 03, 2019 at 09:38:40PM +0200, Jan Ekström wrote:
> > +#define RGB_TO_BGR(c) ((c & 0xff) << 16 | (c & 0xff00) | ((c >> 16) & 
> > 0xff))
>
> c should be protected by a ()
>

First of all, thank you for the technical review.

But yes, it seems like I was too focused on single values it seems (in
which case there is no ambiguity regarding order). Will do.

>
> > +
> > +static void libaribb24_handle_regions(AVCodecContext *avctx, AVSubtitle 
> > *sub)
> > +{
> > +Libaribb24Context *b24 = avctx->priv_data;
> > +const arib_buf_region_t *region = 
> > arib_decoder_get_regions(b24->decoder);
> > +unsigned int profile_font_size = get_profile_font_size(avctx->profile);
> > +AVBPrint buf = { 0 };
> > +
> > +av_bprint_init(&buf, 0, AV_BPRINT_SIZE_UNLIMITED);
> > +
> > +while (region) {
> > +ptrdiff_t region_length = region->p_end - region->p_start;
> > +unsigned int ruby_region =
> > +region->i_fontheight == (profile_font_size / 2);
> > +
> > +// ASS requires us to make the colors BGR, so we convert here
> > +int foreground_bgr_color = RGB_TO_BGR(region->i_foreground_color);
> > +int background_bgr_color = RGB_TO_BGR(region->i_background_color);
> > +
> > +if (region_length < 0) {
> > +av_log(avctx, AV_LOG_ERROR, "Invalid negative region 
> > length!\n");
> > +break;
> > +}
> > +
> > +if (region_length == 0 || (ruby_region && b24->aribb24_skip_ruby)) 
> > {
> > +goto next_region;
> > +}
> > +
> > +// color and alpha
> > +if (foreground_bgr_color != ASS_DEFAULT_COLOR)
> > +av_bprintf(&buf, "{\\1c&H%06x&}", foreground_bgr_color);
> > +
> > +if (region->i_foreground_alpha != 0)
> > +av_bprintf(&buf, "{\\1a&H%02x&}", region->i_foreground_alpha);
> > +
> > +if (background_bgr_color != ASS_DEFAbufULT_BACK_COLOR)
> > +av_bprintf(&buf, "{\\3c&H%06x&}", background_bgr_color);
> > +
> > +if (region->i_background_alpha != 0)
> > +av_bprintf(&buf, "{\\3a&H%02x&}", region->i_background_alpha);
> > +
> > +// font size
> > +if (region->i_fontwidth  != profile_font_size ||
> > +region->i_fontheight != profile_font_size) {
>
> > +av_bprintf(&buf, "{\\fscx%d\\fscy%d}",
> > +   (int)round(((double)region->i_fontwidth /
> > +   (double)profile_font_size) * 100),
> > +   (int)round(((double)region->i_fontheight /
> > +   (double)profile_font_size) * 100));
>
> Please use integers ((u)int64_t) not floating point. This ensures that the 
> results
> are reliably reproducable.
> av_rescale*() may be usefull for this
>

Yes, I was planning to work on this if someone brought it up, but so
far nobody had commented on it.

Will try to post a recalc patch ASAP.

>
>
> > +}
> > +
> > +// TODO: positioning
> > +
> > +av_bprint_append_data(&buf, region->p_start, region_length);
> > +
> > +av_bprintf(&buf, "{\\r}");
> > +
> > +next_region:
> > +region = region->p_next;
> > +}
> > +
> > +av_log(avctx, AV_LOG_DEBUG, "Styled ASS line: %s\n",
> > +   buf.str);
>
> is this function missing malloc failure checks for bprintf* ?
> or is there something that avoids this ?
>

I looked at other subtitle decoders (such as ccaption_dec and libzvbi)
and i didn't notice any checks when adding things to the buffer or
when initializing the bprint context.

Now that you have brought this up, it seems like I was missing
av_bprint_finalize, is this what is meant by this? That seems to be
the only thing where memory allocation is checked with these APIs in
many places. If that is it, I will post a fixup patch ASAP. It did
feel kind of weird not seeing memory allocation checks at each step.

>
> > +ff_ass_add_rect(sub, buf.str, b24->read_order++,
> > +0, NULL, NULL);
> > +
> > +av_bprint_finalize(&buf, NULL);
> > +}
> > +
> > +static int libaribb24_decode(AVCodecContext *avctx, void *data, int 
> > *got_sub_ptr, AVPacket *pkt)
> > +{
> > +Libaribb24Context *b24 = avctx->priv_data;
> > +AVSubtitle *sub = data;
> > +size_t parsed_data_size = 0;
> > +size_t decoded_subtitle_size = 0;
> > +const unsigned char *parsed_data = NULL;
> > +char *decoded_subtitle = NULL;
> > +time_t subtitle_duration = 0;
> > +
> > +if (pkt->size <= 0)
> > +return pkt->size;
> > +
> > +arib_parse_pes(b24->parser, pkt->data, pkt->size);
> > +
> > +parsed_data = arib_parser_get_data(b24->parser,
> > +   &parsed_data_size);
> > +if (!parsed_data || !parsed_data_size) {
> > +av_log(avctx, AV_LOG_DEBUG, "No decode'able data was received from 
> > "
> > +   

Re: [FFmpeg-devel] [PATCH 2/3] libavcodec/cbs: Don't zero twice

2019-02-10 Thread Mark Thompson
On 10/02/2019 23:11, Andreas Rheinhardt wrote:
> Mark Thompson:
>> On 05/02/2019 20:08, Andreas Rheinhardt wrote:
>>>  int ff_cbs_read_extradata(CodedBitstreamContext *ctx,
>>>CodedBitstreamFragment *frag,
>>> -  const AVCodecParameters *par)
>>> +  const AVCodecParameters *par,
>>> +  int reuse)
>>>  {
>>>  int err;
>>>  
>>> -memset(frag, 0, sizeof(*frag));
>>> +if (!reuse)
>>> +memset(frag, 0, sizeof(*frag));
>>>  
>>>  err = cbs_fill_fragment_data(ctx, frag, par->extradata,
>>>   par->extradata_size);
>>> @@ -237,11 +239,12 @@ int ff_cbs_read_extradata(CodedBitstreamContext *ctx,
>>>  
>>>  int ff_cbs_read_packet(CodedBitstreamContext *ctx,
>>> CodedBitstreamFragment *frag,
>>> -   const AVPacket *pkt)
>>> +   const AVPacket *pkt, int reuse)
>>>  {
>>>  int err;
>>>  
>>> -memset(frag, 0, sizeof(*frag));
>>> +if (!reuse)
>>> +memset(frag, 0, sizeof(*frag));
>>>  
>>>  if (pkt->buf) {
>>>  frag->data_ref = av_buffer_ref(pkt->buf);
>>> @@ -266,11 +269,13 @@ int ff_cbs_read_packet(CodedBitstreamContext *ctx,
>>>  
>>>  int ff_cbs_read(CodedBitstreamContext *ctx,
>>>  CodedBitstreamFragment *frag,
>>> -const uint8_t *data, size_t size)
>>> +const uint8_t *data, size_t size,
>>> +int reuse)
>>>  {
>>>  int err;
>>>  
>>> -memset(frag, 0, sizeof(*frag));
>>> +if (!reuse)
>>> +memset(frag, 0, sizeof(*frag));
>>>  
>>>  err = cbs_fill_fragment_data(ctx, frag, data, size);
>>>  if (err < 0)
>>
>> I don't think this patch should be needed.  Can we just document that your 
>> fragment must either be zeroed (so, allocated by av_*allocz() or memset() to 
>> zero) or you've called the ff_cbs_fragment_reset() (whatever the name is) 
>> function before any read call?  It can even check that the user hasn't 
>> messed up by asserting that data, data_size and nb_units are all zero.
> 
> I agree with your suggestion regarding the documentation; but it is
> nevertheless needed to eliminate the memset in the ff_cbs_read
> functions. Otherwise the unit array is leaked.

Yes, sorry.  Indeed still remove the memset, but don't change any of the 
parameters or callers.

Thanks,

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


[FFmpeg-devel] [PATCH 3/4] lavc/libaribb24: add missing type struct members to AVOptions

2019-02-10 Thread Jan Ekström
---
 libavcodec/libaribb24.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavcodec/libaribb24.c b/libavcodec/libaribb24.c
index 43da2b675d..28f20ca767 100644
--- a/libavcodec/libaribb24.c
+++ b/libavcodec/libaribb24.c
@@ -367,9 +367,9 @@ static void libaribb24_flush(AVCodecContext *avctx)
 #define SD AV_OPT_FLAG_SUBTITLE_PARAM | AV_OPT_FLAG_DECODING_PARAM
 static const AVOption options[] = {
 { "aribb24-base-path", "set the base path for the libaribb24 library",
-  OFFSET(aribb24_base_path), AV_OPT_TYPE_STRING, { 0 }, 0, 0, SD },
+  OFFSET(aribb24_base_path), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, SD 
},
 { "aribb24-skip-ruby-text", "skip ruby text blocks during decoding",
-  OFFSET(aribb24_skip_ruby), AV_OPT_TYPE_BOOL, { 1 }, 0, 1, SD },
+  OFFSET(aribb24_skip_ruby), AV_OPT_TYPE_BOOL, { .i64 = 1 }, 0, 1, SD },
 { NULL }
 };
 
-- 
2.20.1

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


[FFmpeg-devel] [PATCH 1/4] lavc/libaribb24: add error handling to region handling

2019-02-10 Thread Jan Ekström
Fixes some rather embarrassing mistakes that somehow passed my
eyes.

* Now catches if memory allocation has failed during bprint usage
  by checking av_bprint_is_complete().
* Now catches if adding an ASS rectangle into an AVSubtitle failed.
* Returns AVERROR_INVALIDDATA if we get an invalid region buffer
  length.
---
 libavcodec/libaribb24.c | 25 ++---
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/libavcodec/libaribb24.c b/libavcodec/libaribb24.c
index bc0255286c..d6cccd117b 100644
--- a/libavcodec/libaribb24.c
+++ b/libavcodec/libaribb24.c
@@ -204,12 +204,13 @@ static int libaribb24_close(AVCodecContext *avctx)
 
 #define RGB_TO_BGR(c) ((c & 0xff) << 16 | (c & 0xff00) | ((c >> 16) & 0xff))
 
-static void libaribb24_handle_regions(AVCodecContext *avctx, AVSubtitle *sub)
+static int libaribb24_handle_regions(AVCodecContext *avctx, AVSubtitle *sub)
 {
 Libaribb24Context *b24 = avctx->priv_data;
 const arib_buf_region_t *region = arib_decoder_get_regions(b24->decoder);
 unsigned int profile_font_size = get_profile_font_size(avctx->profile);
 AVBPrint buf = { 0 };
+int ret = 0;
 
 av_bprint_init(&buf, 0, AV_BPRINT_SIZE_UNLIMITED);
 
@@ -224,6 +225,7 @@ static void libaribb24_handle_regions(AVCodecContext 
*avctx, AVSubtitle *sub)
 
 if (region_length < 0) {
 av_log(avctx, AV_LOG_ERROR, "Invalid negative region length!\n");
+ret = AVERROR_INVALIDDATA;
 break;
 }
 
@@ -264,12 +266,20 @@ next_region:
 region = region->p_next;
 }
 
-av_log(avctx, AV_LOG_DEBUG, "Styled ASS line: %s\n",
-   buf.str);
-ff_ass_add_rect(sub, buf.str, b24->read_order++,
-0, NULL, NULL);
+if (!av_bprint_is_complete(&buf))
+ret = AVERROR(ENOMEM);
+
+if (ret == 0) {
+av_log(avctx, AV_LOG_DEBUG, "Styled ASS line: %s\n",
+   buf.str);
+
+ret = ff_ass_add_rect(sub, buf.str, b24->read_order++,
+  0, NULL, NULL);
+}
 
 av_bprint_finalize(&buf, NULL);
+
+return ret;
 }
 
 static int libaribb24_decode(AVCodecContext *avctx, void *data, int 
*got_sub_ptr, AVPacket *pkt)
@@ -281,6 +291,7 @@ static int libaribb24_decode(AVCodecContext *avctx, void 
*data, int *got_sub_ptr
 const unsigned char *parsed_data = NULL;
 char *decoded_subtitle = NULL;
 time_t subtitle_duration = 0;
+int ret = 0;
 
 if (pkt->size <= 0)
 return pkt->size;
@@ -332,7 +343,7 @@ static int libaribb24_decode(AVCodecContext *avctx, void 
*data, int *got_sub_ptr
avctx->time_base.num, avctx->time_base.den);
 
 if (decoded_subtitle)
-libaribb24_handle_regions(avctx, sub);
+ret = libaribb24_handle_regions(avctx, sub);
 
 *got_sub_ptr = sub->num_rects > 0;
 
@@ -342,7 +353,7 @@ static int libaribb24_decode(AVCodecContext *avctx, void 
*data, int *got_sub_ptr
 // longer and longer...
 arib_finalize_decoder(b24->decoder);
 
-return pkt->size;
+return ret < 0 ? ret : pkt->size;
 }
 
 static void libaribb24_flush(AVCodecContext *avctx)
-- 
2.20.1

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


[FFmpeg-devel] [PATCH 2/4] lavc/libaribb24: protect handled value with parenthesis in RGB_TO_BGR

2019-02-10 Thread Jan Ekström
---
 libavcodec/libaribb24.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/libaribb24.c b/libavcodec/libaribb24.c
index d6cccd117b..43da2b675d 100644
--- a/libavcodec/libaribb24.c
+++ b/libavcodec/libaribb24.c
@@ -202,7 +202,7 @@ static int libaribb24_close(AVCodecContext *avctx)
 return 0;
 }
 
-#define RGB_TO_BGR(c) ((c & 0xff) << 16 | (c & 0xff00) | ((c >> 16) & 0xff))
+#define RGB_TO_BGR(c) (((c) & 0xff) << 16 | ((c) & 0xff00) | (((c) >> 16) & 
0xff))
 
 static int libaribb24_handle_regions(AVCodecContext *avctx, AVSubtitle *sub)
 {
-- 
2.20.1

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


[FFmpeg-devel] [PATCH 4/4] lavc/libaribb24: use integer math to calculate font scaling

2019-02-10 Thread Jan Ekström
---
 libavcodec/libaribb24.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/libavcodec/libaribb24.c b/libavcodec/libaribb24.c
index 28f20ca767..3a59938451 100644
--- a/libavcodec/libaribb24.c
+++ b/libavcodec/libaribb24.c
@@ -249,11 +249,11 @@ static int libaribb24_handle_regions(AVCodecContext 
*avctx, AVSubtitle *sub)
 // font size
 if (region->i_fontwidth  != profile_font_size ||
 region->i_fontheight != profile_font_size) {
-av_bprintf(&buf, "{\\fscx%d\\fscy%d}",
-   (int)round(((double)region->i_fontwidth /
-   (double)profile_font_size) * 100),
-   (int)round(((double)region->i_fontheight /
-   (double)profile_font_size) * 100));
+av_bprintf(&buf, "{\\fscx%"PRId64"\\fscy%"PRId64"}",
+   av_rescale(region->i_fontwidth, 100,
+  profile_font_size),
+   av_rescale(region->i_fontheight, 100,
+  profile_font_size));
 }
 
 // TODO: positioning
-- 
2.20.1

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


Re: [FFmpeg-devel] [PATCH 1/3] libavcodec/cbs: Add ability to keep the units array.

2019-02-10 Thread Andreas Rheinhardt
Mark Thompson:
> On 05/02/2019 20:08, Andreas Rheinhardt wrote:
>> Currently, a fragment's unit array is constantly reallocated during
>> splitting of a packet. This commit adds the ability to keep the unit
>> array by distinguishing between the number of allocated and the number
>> of valid units in the unit array.
>> @@ -548,20 +553,34 @@ static int cbs_insert_unit(CodedBitstreamContext *ctx,
>>  {
>>  CodedBitstreamUnit *units;
>>  
>> -units = av_malloc_array(frag->nb_units + 1, sizeof(*units));
>> -if (!units)
>> -return AVERROR(ENOMEM);
>> +if (frag->nb_units < frag->units_allocated) {
>> +units = frag->units;
>> +
>> +if (position < frag->nb_units)
>> +memmove(units + position + 1, frag->units + position,
>> +(frag->nb_units - position) * sizeof(*units));
>> +} else {
>> +units = av_malloc_array(frag->nb_units + 1, sizeof(*units));
>> +if (!units)
>> +return AVERROR(ENOMEM);
>> +
>> +++frag->units_allocated;
> 
> Given your aim, I wonder whether it would be sensible to have this increase 
> the size by more the one each time - maybe double it or add a small constant 
> (four or eight)?  The unit structures themselves are not particularly large, 
> so this might save a decent number of allocations at the cost of wasting 
> perhaps few hundred bytes of memory.
I thought about this and came to the conclusion that the amount of
allocations that could be saved is not really decent, but negligible:
Usually the size of the unit array stayed in the single digits (e.g.
for H.264 Blurays it's usually four slices, two parameter sets and a
few SEI units). The highest number I found (I didn't try to create
content with artificially many units) were 72 or so from an MPEG-2
Bluray, but that's already an outlier.
Sometimes (when splitting H2645 and when splitting VP9 superframes)
the number of units needed is already known before adding the first
one. But given the low numbers involved, I have not attempted to use
this in order to decrease the number of allocations.
> 
>>  
>> -if (position > 0)
>> -memcpy(units, frag->units, position * sizeof(*units));
>> -if (position < frag->nb_units)
>> -memcpy(units + position + 1, frag->units + position,
>> -   (frag->nb_units - position) * sizeof(*units));
>> +if (position > 0)
>> +memcpy(units, frag->units, position * sizeof(*units));
>> +
>> +if (position < frag->nb_units)
>> +memcpy(units + position + 1, frag->units + position,
>> +   (frag->nb_units - position) * sizeof(*units));
>> +}
>>  
>>  memset(units + position, 0, sizeof(*units));
>>  
>> -av_freep(&frag->units);
>> -frag->units = units;
>> +if (units != frag->units) {
>> +av_free(frag->units);
>> +frag->units = units;
>> +}
>> +
>>  ++frag->nb_units;
>>  
>>  return 0;
>> @@ -652,16 +671,10 @@ int ff_cbs_delete_unit(CodedBitstreamContext *ctx,
>>  
>>  --frag->nb_units;
>>  
>> -if (frag->nb_units == 0) {
>> -av_freep(&frag->units);
>> -
>> -} else {
>> +if (frag->nb_units > 0)
>>  memmove(frag->units + position,
>>  frag->units + position + 1,
>>  (frag->nb_units - position) * sizeof(*frag->units));
>>  
>> -// Don't bother reallocating the unit array.
>> -}
>> -
>>  return 0;
>>  }
>> diff --git a/libavcodec/cbs.h b/libavcodec/cbs.h
>> index 53ac360bb1..229cb129aa 100644
>> --- a/libavcodec/cbs.h
>> +++ b/libavcodec/cbs.h
>> @@ -145,10 +145,19 @@ typedef struct CodedBitstreamFragment {
>>   * and has not been decomposed.
>>   */
>>  int  nb_units;
>> +
>> +/**
>> + * Number of allocated units.
>> + *
>> + * Must always be >= nb_units; designed for internal use by cbs.
>> + */
>> + int  units_allocated;
> 
> I think call this nb_units_allocated for consistent styling.
> 
Ok. My rationale was consistency with H2645Packet.
>> +
>>  /**
>> - * Pointer to an array of units of length nb_units.
>> + * Pointer to an array of units of length units_allocated.
>> + * Only the first nb_units are valid.
>>   *
>> - * Must be NULL if nb_units is zero.
>> + * Must be NULL if units_allocated is zero.
>>   */
>>  CodedBitstreamUnit *units;
>>  } CodedBitstreamFragment;
>> @@ -294,10 +303,12 @@ int ff_cbs_write_packet(CodedBitstreamContext *ctx,
>>  
>>  
>>  /**
>> - * Free all allocated memory in a fragment.
>> + * Free all allocated memory in a fragment except possibly the unit array
>> + * itself. The unit array is only freed if completely is set.
>>   */
>>  void ff_cbs_fragment_uninit(CodedBitstreamContext *ctx,
>> -CodedBitstreamFragment *frag);
>> +CodedBitstreamFragment *frag,
>> +int completely);
> 
> This last parameter se

[FFmpeg-devel] [PATCH V2] tests/api/api-h264-test: Add AV_NOPTS_VALUE check for AVFrame.pkt_dts

2019-02-10 Thread Jun Zhao
Add AV_NOPTS_VALUE check for AVFrame.pkt_dts to avoid print the
pkt_dts as negative number like:
"0,3616613, -9223372036854775808, 1001,  3110400, 0x75e37a65"

Signed-off-by: Jun Zhao 
---
 tests/api/api-h264-test.c |   10 +++---
 1 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/tests/api/api-h264-test.c b/tests/api/api-h264-test.c
index 9fa..55cd6cf 100644
--- a/tests/api/api-h264-test.c
+++ b/tests/api/api-h264-test.c
@@ -131,9 +131,13 @@ static int video_decode_example(const char *input_filename)
 av_log(NULL, AV_LOG_ERROR, "Can't copy image to buffer\n");
 return number_of_written_bytes;
 }
-printf("%d, %10"PRId64", %10"PRId64", %8"PRId64", %8d, 
0x%08lx\n", video_stream,
-fr->pts, fr->pkt_dts, fr->pkt_duration,
-number_of_written_bytes, av_adler32_update(0, (const 
uint8_t*)byte_buffer, number_of_written_bytes));
+
+if (fr->pkt_dts == AV_NOPTS_VALUE)
+printf("%d, %10"PRId64", %s,", video_stream, fr->pts, 
"N/A");
+else
+printf("%d, %10"PRId64", %10"PRId64",", video_stream, 
fr->pts, fr->pkt_dts);
+printf("%8"PRId64", %8d, 0x%08lx\n", fr->pkt_duration,
+   number_of_written_bytes, av_adler32_update(0, (const 
uint8_t*)byte_buffer, number_of_written_bytes));
 }
 av_packet_unref(&pkt);
 av_init_packet(&pkt);
-- 
1.7.1

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


Re: [FFmpeg-devel] [PATCH] doc/faq: update macOS and URLs

2019-02-10 Thread Gyan



On 10-02-2019 10:20 PM, Reto Kromer wrote:

Best regards, Reto


Pushed as 6174686bc346e24fd146a725a97d77e571ebf5b4

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