Re: [FFmpeg-devel] [PATCH] libavdevice/decklink: 32 bit audio support

2017-10-19 Thread Douglas Marsh 

On 2017-10-19 20:08, Douglas Marsh wrote:


Is that a commit (1e5ff78fec9b13eccac9a96acc358bbfd6a7015d) I should
check out on my Decklink and try or is there another one I should try?

Will try to get to testing it this weekend.



Nevermind I found this:

* commit 89cc48551bbe9f147ba9f4ca3821a35797cf7b47
| Author: Dave Rice 
| Date:   Wed Oct 18 15:21:46 2017 -0400
|
| avdevice/decklink_dec: 32 bit audio support
|
| Signed-off-by: Marton Balint 


I'll merge that with a local branch I forked for testing purposes 
(marking a known place for testing).


--Doug (dx9s)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] libavdevice/decklink: 32 bit audio support

2017-10-19 Thread Douglas Marsh 

On 2017-10-18 12:23, Dave Rice wrote:

[...]
Updated.

From 1e5ff78fec9b13eccac9a96acc358bbfd6a7015d Mon Sep 17 00:00:00 2001
From: Dave Rice 
Date: Wed, 18 Oct 2017 15:21:46 -0400
Subject: [PATCH] libavdevice/decklink: 32 bit audio support



Is that a commit (1e5ff78fec9b13eccac9a96acc358bbfd6a7015d) I should 
check out on my Decklink and try or is there another one I should try?


Will try to get to testing it this weekend.

--Doug (dx9s)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] libavcodec/h263dec.c: Duplicate the last decoded frame when xvid marks the packet as skipped.

2017-10-19 Thread Thierry Foucu 
On Thu, Oct 19, 2017 at 3:43 PM, Michael Niedermayer  wrote:

> On Thu, Oct 19, 2017 at 09:51:05AM -0700, Thierry Foucu wrote:
> > Instead of returning nothing when we detect the xvid skipped frame, we
> > could return the last decoded frame, if we do have one.
>
> FRAME_SKIPPED is not limited to xvid (packed frames). There are a few
> other pathes that return it.
>
>
What if i change the test to check for mpeg4 video codec, something like

 if (ret == FRAME_SKIPPED) {
+if (CONFIG_MPEG4_DECODER && actvx->codec_id == AV_CODEC_ID_MPEG4
&& s->next_picture_ptr) {
+if ((ret = av_frame_ref(pict, s->next_picture_ptr->f)) < 0) {
+return ret;
+}
+*got_frame = 1;
+}
 return get_consumed_bytes(s, buf_size);
+}

or should i do something specific for xvid bug? I could return another
value for the xvid (packed frames)  here
https://github.com/FFmpeg/FFmpeg/blob/master/libavcodec/mpeg4videodec.c#L2590

and check for it instead of the frame_skipped.

For standard mpeg4video, outputing the next frame smells like violating
> the spec.
>
>
i'm not output the next frame, i'm output the last decoded frame

The problem we have is that we have video with the last 5 second of it
being xvid packed frames.
When we demux the video, we assume the video to be 5 second long, but when
we decode it, we have only the first 2 frames, then all the others are
packed frames and so, the output is 2 frames long (instead of 5 seconds)

Here are the options we were thinking:
* if packed frames, duplicate and return the last frame if we  do have it
(this is this patch)
* increase the next PTS even if we have packed frames, so the last PTS is
passed down correctly to the filter chain when closing it and using the fps
filter, we could pad the last 5 second missing.



> [...]
>
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Observe your enemies, for they first find out your faults. -- Antisthenes
>
> ___
> 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] libavcodec/h263dec.c: Duplicate the last decoded frame when xvid marks the packet as skipped.

2017-10-19 Thread Michael Niedermayer 
On Thu, Oct 19, 2017 at 09:51:05AM -0700, Thierry Foucu wrote:
> Instead of returning nothing when we detect the xvid skipped frame, we
> could return the last decoded frame, if we do have one.

FRAME_SKIPPED is not limited to xvid (packed frames). There are a few
other pathes that return it.

For standard mpeg4video, outputing the next frame smells like violating
the spec.

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

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


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

Re: [FFmpeg-devel] [PATCH] Fix for paletteuse to support transparency

2017-10-19 Thread Carl Eugen Hoyos 
2017-10-20 0:00 GMT+02:00 Bjorn Roche :
> The palettes generated by palettegen include one transparent color.
> This patch enables paletteuse to identify the transparency in incoming
> video and tag transparent pixels on outgoing video.
>
> This requires tracking the transparency index in the palette,
> establishing an alpha threshold below which a pixel is considered
> transparent and above which the pixel is considered opaque, and
> additional changes to track the alpha value throught the conversion
> process. If the format of a color variable has changed in this patch,
> an effort was also made to change the variable name for clarity.
>
> This change is a partial fix for https://trac.ffmpeg.org/ticket/4443
> However, animated GIFs are still output incorrectly due to a bug
> in gif optimization which does not correctly handle transparency.

If Clément doesn't comment, I will probably push this in a few days.

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

Re: [FFmpeg-devel] [PATCH] Fix quadratic memory use in ff_h2645_extract_rbsp() when multiple NALUs exist in packet.

2017-10-19 Thread Carl Eugen Hoyos 
2017-10-19 20:46 GMT+02:00 Nikolas Bowe :
> Found via fuzzing.
> /tmp/poc is a 1 MB mpegts file generated via fuzzing, where 1 packet has many 
> NALUs
> Before this change:
>   $ /usr/bin/time -f "\t%M Max Resident Set Size (Kb)"  ./ffprobe /tmp/poc 
> 2>&1 | tail -n 1
> 2158192 Max Resident Set Size (Kb)
> After this change:
>   $ /usr/bin/time -f "\t%M Max Resident Set Size (Kb)"  ./ffprobe /tmp/poc 
> 2>&1 | tail -n 1
> 1046812 Max Resident Set Size (Kb)

This does not look like a fix for a "quadratic" memory consumption or
do I misunderstand?

Does the patch have a measurable speed impact?

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

[FFmpeg-devel] [PATCH] Fix for paletteuse to support transparency

2017-10-19 Thread Bjorn Roche 
The palettes generated by palettegen include one transparent color.
This patch enables paletteuse to identify the transparency in incoming
video and tag transparent pixels on outgoing video.

This requires tracking the transparency index in the palette,
establishing an alpha threshold below which a pixel is considered
transparent and above which the pixel is considered opaque, and
additional changes to track the alpha value throught the conversion
process. If the format of a color variable has changed in this patch,
an effort was also made to change the variable name for clarity.

This change is a partial fix for https://trac.ffmpeg.org/ticket/4443
However, animated GIFs are still output incorrectly due to a bug
in gif optimization which does not correctly handle transparency.
---
 libavfilter/vf_paletteuse.c | 210 
 1 file changed, 132 insertions(+), 78 deletions(-)

diff --git a/libavfilter/vf_paletteuse.c b/libavfilter/vf_paletteuse.c
index 79a0672891..fa5897b7f3 100644
--- a/libavfilter/vf_paletteuse.c
+++ b/libavfilter/vf_paletteuse.c
@@ -56,7 +56,7 @@ enum diff_mode {
 };
 
 struct color_node {
-uint8_t val[3];
+uint8_t val[4];
 uint8_t palette_id;
 int split;
 int left_id, right_id;
@@ -86,6 +86,8 @@ typedef struct PaletteUseContext {
 struct cache_node cache[CACHE_SIZE];/* lookup cache */
 struct color_node map[AVPALETTE_COUNT]; /* 3D-Tree (KD-Tree with K=3) for 
reverse colormap */
 uint32_t palette[AVPALETTE_COUNT];
+int transparency_index; /* index in the palette of transparency. -1 if 
there isn't a transparency. */
+int trans_thresh;
 int palette_loaded;
 int dither;
 int new;
@@ -116,6 +118,7 @@ static const AVOption paletteuse_options[] = {
 { "bayer_scale", "set scale for bayer dithering", OFFSET(bayer_scale), 
AV_OPT_TYPE_INT, {.i64=2}, 0, 5, FLAGS },
 { "diff_mode",   "set frame difference mode", OFFSET(diff_mode),   
AV_OPT_TYPE_INT, {.i64=DIFF_MODE_NONE}, 0, NB_DIFF_MODE-1, FLAGS, "diff_mode" },
 { "rectangle", "process smallest different rectangle", 0, 
AV_OPT_TYPE_CONST, {.i64=DIFF_MODE_RECTANGLE}, INT_MIN, INT_MAX, FLAGS, 
"diff_mode" },
+{ "threshold", "set the alpha threshold for transparency. Above this 
threshold, pixels are considered opaque, below they are considered 
transparent.", OFFSET(trans_thresh), AV_OPT_TYPE_INT, {.i64=128}, 0, 255, },
 
 /* following are the debug options, not part of the official API */
 { "debug_kdtree", "save Graphviz graph of the kdtree in specified file", 
OFFSET(dot_filename), AV_OPT_TYPE_STRING, {.str=NULL}, CHAR_MIN, CHAR_MAX, 
FLAGS },
@@ -157,34 +160,44 @@ static int query_formats(AVFilterContext *ctx)
 
 static av_always_inline int dither_color(uint32_t px, int er, int eg, int eb, 
int scale, int shift)
 {
-return av_clip_uint8((px >> 16 & 0xff) + ((er * scale) / (1<> 24  ) << 24
+ | av_clip_uint8((px >> 16 & 0xff) + ((er * scale) / (1<>  8 & 0xff) + ((eg * scale) / (1<= trans_thresh && c2[0] >= trans_thresh) {
+return dr*dr + dg*dg + db*db;
+} else {
+return max_diff;
+}
 }
 
-static av_always_inline uint8_t colormap_nearest_bruteforce(const uint32_t 
*palette, const uint8_t *rgb)
+static av_always_inline uint8_t colormap_nearest_bruteforce(const uint32_t 
*palette, const uint8_t *argb, const int trans_thresh)
 {
 int i, pal_id = -1, min_dist = INT_MAX;
 
 for (i = 0; i < AVPALETTE_COUNT; i++) {
 const uint32_t c = palette[i];
 
-if ((c & 0xff00) == 0xff00) { // ignore transparent entry
-const uint8_t palrgb[] = {
+if ( c >> 24 >= trans_thresh ) { // ignore transparent entry
+const uint8_t palargb[] = {
+palette[i]>>24 & 0xff,
 palette[i]>>16 & 0xff,
 palette[i]>> 8 & 0xff,
 palette[i] & 0xff,
 };
-const int d = diff(palrgb, rgb);
+const int d = diff(palargb, argb, trans_thresh);
 if (d < min_dist) {
 pal_id = i;

Re: [FFmpeg-devel] [PATCH 1/4] lavf: add MP1 muxer

2017-10-19 Thread Michael Niedermayer 
On Thu, Oct 19, 2017 at 02:39:45AM -0500, Rodger Combs wrote:
> ---
>  libavformat/Makefile |  1 +
>  libavformat/allformats.c |  1 +
>  libavformat/rawenc.c | 13 +
>  3 files changed, 15 insertions(+)

please add a fate test for this too

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

Republics decline into democracies and democracies degenerate into
despotisms. -- Aristotle


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

Re: [FFmpeg-devel] [RFC] v4l2_m2m: Fix races around freeing data on close

2017-10-19 Thread Jorge Ramirez 

On 10/19/2017 07:55 PM, Jorge Ramirez wrote:

On 10/19/2017 11:49 AM, Mark Thompson wrote:

On 19/10/17 08:28, Jorge Ramirez wrote:

On 10/19/2017 02:10 AM, Mark Thompson wrote:

Refcount all of the context information.
---
As discussed in the other thread, something like this.  We move 
most of the context into a refcounted buffer and 
AVCodecContext.priv_data is left as a stub holding a reference to it.
um, ok ... please could you send a patch so I can apply it? I see 
several problems in v4l2_free_buffer.
What goes wrong?  It applies fine for me on current head 
(f4090940bd3024e69d236257d327f11d1e496229).


yes my bad.



To sum up the RFC, instead of using private_data to place the 
codec's context, it uses private data to place a _pointer_ to an 
allocated codec context "just" so it wont be deallocated after the 
codec is closed (codec close frees the private_data)


Personally I think adding AV_CODEC_FLAG2_PRESERVE_PRIVDATA_ONCLOSE 
and use private_data to hold the codec context is a cleaner/simpler 
approach.


- the codec requests private_data with a size (sizeof(type))
- the code explicitly informs the API whether all memory will be 
released on close or it will preserve it.
- All APIs in ffmpeg with this sort of private data field use them in 
the same way - they are allocated at create/alloc time (with the 
given size, for AVOptions), and freed at close/destroy time.
- Using the well-tested reference-counted buffer implementation is 
IMO strongly preferable to making ad-hoc synchronisation with atomics 
and semaphores.
- All other decoders use the reference-counted buffer approach (e.g. 
look at rkmpp for a direct implementation, the hwaccels all do it via 
hwcontext).


ok so I guess there is no point to discuss further what I tried to put 
forward (I could provide my implementation to compare against this RFC 
- it is just a handful of lines of code - but I guess there is no 
point given that everyone is comfortable with the current way of doing 
things.).


so let's make this work then and fix the SIGSEGV present in master 
asap (btw this RFC also seg-fault when the above is applied)


diff --git a/libavcodec/v4l2_m2m_dec.c b/libavcodec/v4l2_m2m_dec.c
index 831fd81..1dd8cf0 100644
--- a/libavcodec/v4l2_m2m_dec.c
+++ b/libavcodec/v4l2_m2m_dec.c
@@ -176,8 +176,8 @@ static av_cold int v4l2_decode_init(AVCodecContext 
*avctx)
  * by the v4l2 driver; this event will trigger a full pipeline 
reconfig and

  * the proper values will be retrieved from the kernel driver.
  */
-output->height = capture->height = avctx->coded_height;
-output->width = capture->width = avctx->coded_width;
+output->height = capture->height = 0; //avctx->coded_height;
+output->width = capture->width = 0; //avctx->coded_width;

 output->av_codec_id = avctx->codec_id;
 output->av_pix_fmt  = AV_PIX_FMT_NONE;


also looking at your code I think we also need:

[jramirez@igloo libavcodec (patches/m6 *$)]$ git diff v4l2_buffers.c
diff --git a/libavcodec/v4l2_buffers.c b/libavcodec/v4l2_buffers.c
index 9831bdb..8dec56d 100644
--- a/libavcodec/v4l2_buffers.c
+++ b/libavcodec/v4l2_buffers.c
@@ -207,15 +207,19 @@ static void v4l2_free_buffer(void *opaque, 
uint8_t *unused)

 V4L2Buffer* avbuf = opaque;
 V4L2m2mContext *s = buf_to_m2mctx(avbuf);

-atomic_fetch_sub_explicit(>refcount, 1, memory_order_acq_rel);
-if (s->reinit) {
-if (!atomic_load(>refcount))
-sem_post(>refsync);
-} else if (avbuf->context->streamon) {
-ff_v4l2_buffer_enqueue(avbuf);
-}
+av_log(logger(avbuf), AV_LOG_DEBUG, "free capture %d\n", 
avbuf->buf.index);


 if (atomic_fetch_sub(>context_refcount, 1) == 1) {
+atomic_fetch_sub_explicit(>refcount, 1, 
memory_order_acq_rel);

+
+if (s->reinit) {
+if (!atomic_load(>refcount))
+sem_post(>refsync);
+} else if (avbuf->context->streamon) {
+av_log(logger(avbuf), AV_LOG_DEBUG, "queue capture %d\n", 
avbuf->buf.index);

+ff_v4l2_buffer_enqueue(avbuf);
+}
+
 av_buffer_unref(>context_ref);
 }
 }

I tested the encoder and it seems to work properly (havent checked 
with valgrind but all frames are properly encoded)


how do you want to proceed?
will you create a branch somewhere and we work on this or should I 
take it and evolve it or will you do it all? thanks!





fyi

testing the h264 m2m encoder

==10241== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==10241== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==10241== Command: /home/linaro/Src/git.zoltan.ffmpeg/ffmpeg_g -report 
-threads 1 -v 55 -f rawvideo -pix_fmt nv12 -s:v 1280:720 -r 25 -i 
/home/linaro/Videos/raw/freeway.yuv -c:v h264_v4l2m2m out/out.h264.mp4

==10241==

...
...

Input file #0 (/home/linaro/Videos/raw/freeway.yuv):
  Input stream #0:0 (video): 232 packets read (320716800 bytes); 232 
frames decoded;

  Total: 232 packets

Re: [FFmpeg-devel] libavcodec/als: remove check for predictor order of a block

2017-10-19 Thread Ronald S. Bultje 
Hi,

On Thu, Oct 19, 2017 at 4:03 PM, Umair Khan  wrote:

> I tried decoding the file in both the cases and I don't see any
> address related error in the console while decoding. Following is the
> output after I apply the patch :-
>
[..]

> Is there something which I'm missing?
>

You need to run under valgrind or compile with address sanitizer support:
configure --toolchain=gcc-asan or --toolchain=clang-asan, depending on the
name of clang on your system.

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

Re: [FFmpeg-devel] [PATCH] libavformat: not treat 0 as EOF

2017-10-19 Thread Nicolas George 
L'octidi 28 vendémiaire, an CCXXVI, Michael Niedermayer a écrit :
> any reason why you dont apply it, if you think its correct ?

Good question. Just did it.

Regards,

-- 
  Nicolas George


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

Re: [FFmpeg-devel] libavcodec/als: remove check for predictor order of a block

2017-10-19 Thread Umair Khan 
Hi Ronald,

On Sun, Oct 15, 2017 at 12:21 AM, Ronald S. Bultje  wrote:
> Hi Umair,
>
> On Sat, Oct 14, 2017 at 1:43 PM, Umair Khan  wrote:
>
>> On Sat, Oct 14, 2017 at 8:02 PM, Ronald S. Bultje 
>> wrote:
>> > Hi Umair,
>> >
>> > On Sat, Oct 14, 2017 at 9:59 AM, Umair Khan  wrote:
>> >
>> >> I tested the file which Michael sent. The thing is that I'm getting
>> >> error in decoding that file in both the cases, with or without the
>> >> patch. I will begin debugging this issue, however I think the file
>> >> which Michael sent has got nothing to do with the patch in this
>> >> thread.
>> >>
>> >
>> > I don't think the file is meant to be decoded correctly, it's a specially
>> > crafted file to demonstrate that certain codepaths (triggered by files
>> such
>> > as this) can be used to trigger unwanted behaviour (overreads,
>> overwrites,
>> > etc.). Eventually, combinations of such files can be used to break into
>> > your system with specially crafted media files (yes, really).
>> >
>> > Your patch introduces such a security issue (since it's triggered by the
>> > file after, but not before the patch). This must be fixed before the
>> patch
>> > can be committed.
>>
>> Okay. You mean the file isn't supposed to be decoded and that the als
>> decoder should output the proper error message instead of breaking at
>> a random point. Am I getting it correct?
>>
>
> More specifically: after your patch, you'll notice that address sanitizer
> (clang -fsanitize=address) or valgrind output warnings when decoding this
> file. These warnings should be tracked down and fixed.

I tried decoding the file in both the cases and I don't see any
address related error in the console while decoding. Following is the
output after I apply the patch :-

➜  FFmpeg git:(master) ✗ ffmpeg -i
~/Downloads/abd3c041acbcb816be113455d138166b-asan_heap-oob_b11634_3707_cov_1707137151_als_05_2ch48k16b.mp4
out.mp4
ffmpeg version N-87928-g247281e805 Copyright (c) 2000-2017 the FFmpeg developers
  built with Apple LLVM version 9.0.0 (clang-900.0.38)
  configuration: --prefix=/usr/local --enable-gpl --enable-nonfree
--enable-libass --enable-libfdk-aac --enable-libfreetype
--enable-libmp3lame --enable-libtheora --enable-libvorbis
--enable-libvpx --enable-libx264 --enable-libx265 --enable-libopus
--enable-libxvid
  libavutil  55. 79.100 / 55. 79.100
  libavcodec 57.108.101 / 57.108.101
  libavformat57. 84.101 / 57. 84.101
  libavdevice57. 11.100 / 57. 11.100
  libavfilter 6.108.100 /  6.108.100
  libswscale  4.  9.100 /  4.  9.100
  libswresample   2. 10.100 /  2. 10.100
  libpostproc54.  8.100 / 54.  8.100
Guessed Channel Layout for Input Stream #0.0 : stereo
Input #0, mov,mp4,m4a,3gp,3g2,mj2, from
'/Users/mohammadumair/Downloads/abd3c041acbcb816be113455d138166b-asan_heap-oob_b11634_3707_cov_1707137151_als_05_2ch48k16b.mp4':
  Metadata:
major_brand : mt42
minor_version   : 0
compatible_brands: mp42i{om
creation_time   : 2008-01-21T14:37:08.00Z
  Duration: 00:00:14.81, start: 0.00, bitrate: 436 kb/s
Stream #0:0(und): Audio: mp4als (mp4a / 0x6134706D), 48000 Hz,
stereo, s16, 435 kb/s (default)
Metadata:
  creation_time   : 2008-01-24T15:26:12.00Z
File 'out.mp4' already exists. Overwrite ? [y/N] y
Stream mapping:
  Stream #0:0 -> #0:0 (mp4als (als) -> aac (native))
Press [q] to stop, [?] for help
[als @ 0x7fc2df001200] r overflow
[als @ 0x7fc2df001200] Reading frame data failed. Skipping RA unit.
Output #0, mp4, to 'out.mp4':
  Metadata:
major_brand : mt42
minor_version   : 0
compatible_brands: mp42i{om
encoder : Lavf57.84.101
Stream #0:0(und): Audio: aac (LC) (mp4a / 0x6134706D), 48000 Hz,
stereo, fltp (16 bit), 128 kb/s (default)
Metadata:
  creation_time   : 2008-01-24T15:26:12.00Z
  encoder : Lavc57.108.101 aac
size=   2kB time=00:00:00.06 bitrate= 217.0kbits/s speed=7.48x
video:0kB audio:1kB subtitle:0kB other streams:0kB global headers:0kB
muxing overhead: 90.142387%
[aac @ 0x7fc2df002a00] Qavg: 16482.379


Is there something which I'm missing?

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

Re: [FFmpeg-devel] [PATCH] libavformat: not treat 0 as EOF

2017-10-19 Thread Michael Niedermayer 
On Thu, Oct 19, 2017 at 04:22:33PM +0200, Nicolas George wrote:
> Le sextidi 26 vendémiaire, an CCXXVI, Michael Niedermayer a écrit :
> > Seems to work, no more comments from me
> 
> I think this is correct too.

any reason why you dont apply it, if you think its correct ?

[...]


-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

While the State exists there can be no freedom; when there is freedom there
will be no State. -- Vladimir Lenin


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

Re: [FFmpeg-devel] [PATCH] avcodec/v4l2: fix access to priv_data after codec close.

2017-10-19 Thread Michael Niedermayer 
On Thu, Oct 19, 2017 at 09:01:36AM +0200, Jorge Ramirez wrote:
> On 10/18/2017 09:40 PM, Michael Niedermayer wrote:
> >On Wed, Oct 18, 2017 at 09:46:40AM +0200, Jorge Ramirez wrote:
> >>On 10/18/2017 12:34 AM, Mark Thompson wrote:
>   int ff_v4l2_m2m_codec_end(AVCodecContext *avctx)
>   {
> -V4L2m2mContext* s = avctx->priv_data;
> -int ret;
> +V4L2m2mContext *m2m, *s = avctx->priv_data;
> +int i, ret;
>   ret = ff_v4l2_context_set_status(>output, VIDIOC_STREAMOFF);
>   if (ret)
> @@ -325,12 +325,31 @@ int ff_v4l2_m2m_codec_end(AVCodecContext *avctx)
>   av_log(avctx, AV_LOG_ERROR, "VIDIOC_STREAMOFF %s\n", 
>  s->capture.name);
>   ff_v4l2_context_release(>output);
> +sem_destroy(>refsync);
> -if (atomic_load(>refcount))
> -av_log(avctx, AV_LOG_ERROR, "ff_v4l2m2m_codec_end leaving 
> pending buffers\n");
> +if (atomic_load(>refcount)) {
> +av_log(avctx, AV_LOG_DEBUG, "ff_v4l2m2m_codec_end, referenced 
> buffers %d\n", atomic_load(>refcount));
> +
> +/* We are about to free the private data while the user still 
> has references to the buffers.
> + * This means that when the user releases those buffers, the 
> v4l2_free_buffer callback will be pointing to free'd memory.
> + * Duplicate the m2m context and update the buffers.
> + */
> +m2m = av_mallocz(sizeof(*m2m));
> +if (m2m) {
> +memcpy(m2m, s, sizeof(V4L2m2mContext));
> +for (i = 0; i < s->capture.num_buffers; i++)
> +s->capture.buffers[i].m2m = m2m;
> +
> +return 0;
> +}
> >>>No.
> >>>
> >>>The user can call av_buffer_unref() and therefore v4l2_free_buffer() 
> >>>asynchronously on another thread while this manoeuvre is in progress.
> >>>
> >>right. as a matter of fact, this synchronization can not be done
> >>using private data (the opaque structure passed to the callback must
> >>contain all the required information (and updated!) at any time). On
> >>top of this being hard to implement would complicate the current
> >>code quite a bit making it also very hard to maintain. So agreed.
> >>
> >>So back to a previous proposal, would the flag below be acceptable? [1]
> >>
> >>Seems sensible to me to give responsibility of the private data
> >>deallocation to the codec itself independently of the close function
> >>since the buffers allocated by the codec controlled by a separate
> >>thread will need access to that data.
> >>
> >>Put differently since the close function can not be made responsible
> >>for deallocating mmap/queued memory in V4L2 (or even closing the
> >>kernel driver) when the user keeps buffers, it should not be allowed
> >>to delete the codec private data since in my view it forces the
> >>design to take a lot of unnecessary actions for no real reason.
> >>
> >>With the proposal below, the code remains cleaner and free of race
> >>conditions.
> >>
> >>[1] ..
> >>
> >>diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> >>index 52cc5b0..0a3ca0c 100644
> >>--- a/libavcodec/avcodec.h
> >>+++ b/libavcodec/avcodec.h
> >>@@ -982,6 +982,11 @@ typedef struct RcOverride{
> >>   * Do not reset ASS ReadOrder field on flush (subtitles decoding)
> >>   */
> >>  #define AV_CODEC_FLAG2_RO_FLUSH_NOOP  (1 << 30)
> >>+/**
> >>+ * Closing the codec doesnt release the context priv_data (it
> >>becomes the codec
> >>+ * responsibility)
> >>+ */
> >>+#define AV_CODEC_FLAG2_PRESERVE_PRIVDATA_ONCLOSE  (1 << 31)
> >>
> >>  /* Unsupported options :
> >>   *  Syntax Arithmetic coding (SAC)
> >>diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> >>index 9551f31..b6c5adf 100644
> >>--- a/libavcodec/utils.c
> >>+++ b/libavcodec/utils.c
> >>@@ -1211,7 +1211,9 @@ av_cold int avcodec_close(AVCodecContext *avctx)
> >>  if (avctx->priv_data && avctx->codec && avctx->codec->priv_class)
> >>  av_opt_free(avctx->priv_data);
> >>  av_opt_free(avctx);
> >>-av_freep(>priv_data);
> >>+if (!(avctx->flags2 & AV_CODEC_FLAG2_PRESERVE_PRIVDATA_ONCLOSE))
> >>+av_freep(>priv_data);
> >
> >This may be solving a similar issue as our udp.c cleanup with its
> >thread. Not sure, i didnt follow this discussion closely.
> >just wanted to mention as one migt benefit from the other if its the
> >same issue.
> >
> >Another thing that came to my mind, if the context (and other resources)
> >are deallocated with a delay beyond closing. Is this still correct if
> >the process terminates before or in the middle.
> >Or is that preented somehow ?
> 
> sorry not sure I follow, since the buffer references also belong to
> the process closing the process would end up closing the file
> descriptor (ie v4l2 driver).
> the driver would have to do its internal house keeping (release its
> queued buffers and so on).
> I dont think this would be an issue.

[FFmpeg-devel] [PATCH] Fix quadratic memory use in ff_h2645_extract_rbsp() when multiple NALUs exist in packet.

2017-10-19 Thread Nikolas Bowe 
Found via fuzzing.
/tmp/poc is a 1 MB mpegts file generated via fuzzing, where 1 packet has many 
NALUs
Before this change:
  $ /usr/bin/time -f "\t%M Max Resident Set Size (Kb)"  ./ffprobe /tmp/poc 2>&1 
| tail -n 1
2158192 Max Resident Set Size (Kb)
After this change:
  $ /usr/bin/time -f "\t%M Max Resident Set Size (Kb)"  ./ffprobe /tmp/poc 2>&1 
| tail -n 1
1046812 Max Resident Set Size (Kb)
---
 libavcodec/h2645_parse.c | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/libavcodec/h2645_parse.c b/libavcodec/h2645_parse.c
index b0d9ff66f0..e77689f347 100644
--- a/libavcodec/h2645_parse.c
+++ b/libavcodec/h2645_parse.c
@@ -32,7 +32,7 @@
 int ff_h2645_extract_rbsp(const uint8_t *src, int length,
   H2645NAL *nal, int small_padding)
 {
-int i, si, di;
+int i, si, di, nsc;
 uint8_t *dst;
 int64_t padding = small_padding ? 0 : MAX_MBPAIR_SIZE;
 
@@ -91,8 +91,17 @@ int ff_h2645_extract_rbsp(const uint8_t *src, int length,
 } else if (i > length)
 i = length;
 
+// Find next NAL start code, if present, to reduce rbsp_buffer size when
+// multiple NALUs.
+for (nsc = i; nsc + 2 < length; nsc++) {
+if (src[nsc] == 0 && src[nsc + 1] == 0 && src[nsc + 2] == 1)
+  break;
+}
+if (nsc + 2 == length)
+nsc = length;
+
 av_fast_padded_malloc(>rbsp_buffer, >rbsp_buffer_size,
-  length + padding);
+  nsc + padding);
 if (!nal->rbsp_buffer)
 return AVERROR(ENOMEM);
 
-- 
2.15.0.rc1.287.g2b38de12cc-goog

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

Re: [FFmpeg-devel] [RFC] v4l2_m2m: Fix races around freeing data on close

2017-10-19 Thread Jorge Ramirez 

On 10/19/2017 11:49 AM, Mark Thompson wrote:

On 19/10/17 08:28, Jorge Ramirez wrote:

On 10/19/2017 02:10 AM, Mark Thompson wrote:

Refcount all of the context information.
---
As discussed in the other thread, something like this.  We move most of the 
context into a refcounted buffer and AVCodecContext.priv_data is left as a stub 
holding a reference to it.

um, ok ... please could you send a patch so I can apply it? I see several 
problems in v4l2_free_buffer.

What goes wrong?  It applies fine for me on current head 
(f4090940bd3024e69d236257d327f11d1e496229).


yes my bad.




To sum up the RFC, instead of using private_data to place the codec's context, it uses 
private data to place a _pointer_ to an allocated codec context "just" so it 
wont be deallocated after the codec is closed (codec close frees the private_data)

Personally I think adding AV_CODEC_FLAG2_PRESERVE_PRIVDATA_ONCLOSE and use 
private_data to hold the codec context is a cleaner/simpler approach.

- the codec requests private_data with a size (sizeof(type))
- the code explicitly informs the API whether all memory will be released on 
close or it will preserve it.

- All APIs in ffmpeg with this sort of private data field use them in the same 
way - they are allocated at create/alloc time (with the given size, for 
AVOptions), and freed at close/destroy time.
- Using the well-tested reference-counted buffer implementation is IMO strongly 
preferable to making ad-hoc synchronisation with atomics and semaphores.
- All other decoders use the reference-counted buffer approach (e.g. look at 
rkmpp for a direct implementation, the hwaccels all do it via hwcontext).


ok so I guess there is no point to discuss further what I tried to put 
forward (I could provide my implementation to compare against this RFC - 
it is just a handful of lines of code - but I guess there is no point 
given that everyone is comfortable with the current way of doing things.).


so let's make this work then and fix the SIGSEGV present in master asap 
(btw this RFC also seg-fault when the above is applied)


diff --git a/libavcodec/v4l2_m2m_dec.c b/libavcodec/v4l2_m2m_dec.c
index 831fd81..1dd8cf0 100644
--- a/libavcodec/v4l2_m2m_dec.c
+++ b/libavcodec/v4l2_m2m_dec.c
@@ -176,8 +176,8 @@ static av_cold int v4l2_decode_init(AVCodecContext 
*avctx)
  * by the v4l2 driver; this event will trigger a full pipeline 
reconfig and

  * the proper values will be retrieved from the kernel driver.
  */
-output->height = capture->height = avctx->coded_height;
-output->width = capture->width = avctx->coded_width;
+output->height = capture->height = 0; //avctx->coded_height;
+output->width = capture->width = 0; //avctx->coded_width;

 output->av_codec_id = avctx->codec_id;
 output->av_pix_fmt  = AV_PIX_FMT_NONE;


also looking at your code I think we also need:

[jramirez@igloo libavcodec (patches/m6 *$)]$ git diff v4l2_buffers.c
diff --git a/libavcodec/v4l2_buffers.c b/libavcodec/v4l2_buffers.c
index 9831bdb..8dec56d 100644
--- a/libavcodec/v4l2_buffers.c
+++ b/libavcodec/v4l2_buffers.c
@@ -207,15 +207,19 @@ static void v4l2_free_buffer(void *opaque, uint8_t 
*unused)

 V4L2Buffer* avbuf = opaque;
 V4L2m2mContext *s = buf_to_m2mctx(avbuf);

-atomic_fetch_sub_explicit(>refcount, 1, memory_order_acq_rel);
-if (s->reinit) {
-if (!atomic_load(>refcount))
-sem_post(>refsync);
-} else if (avbuf->context->streamon) {
-ff_v4l2_buffer_enqueue(avbuf);
-}
+av_log(logger(avbuf), AV_LOG_DEBUG, "free capture %d\n", 
avbuf->buf.index);


 if (atomic_fetch_sub(>context_refcount, 1) == 1) {
+atomic_fetch_sub_explicit(>refcount, 1, memory_order_acq_rel);
+
+if (s->reinit) {
+if (!atomic_load(>refcount))
+sem_post(>refsync);
+} else if (avbuf->context->streamon) {
+av_log(logger(avbuf), AV_LOG_DEBUG, "queue capture %d\n", 
avbuf->buf.index);

+ff_v4l2_buffer_enqueue(avbuf);
+}
+
 av_buffer_unref(>context_ref);
 }
 }

I tested the encoder and it seems to work properly (havent checked with 
valgrind but all frames are properly encoded)


how do you want to proceed?
will you create a branch somewhere and we work on this or should I take 
it and evolve it or will you do it all? thanks!




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

[FFmpeg-devel] [PATCH] libavcodec/h263dec.c: Duplicate the last decoded frame when xvid marks the packet as skipped.

2017-10-19 Thread Thierry Foucu 
Instead of returning nothing when we detect the xvid skipped frame, we
could return the last decoded frame, if we do have one.
---
 libavcodec/h263dec.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/libavcodec/h263dec.c b/libavcodec/h263dec.c
index bcb2b08bb0..e1667bcbc1 100644
--- a/libavcodec/h263dec.c
+++ b/libavcodec/h263dec.c
@@ -507,8 +507,15 @@ retry:
 s->height= avctx->coded_height;
 }
 }
-if (ret == FRAME_SKIPPED)
+if (ret == FRAME_SKIPPED) {
+if (s->next_picture_ptr) {
+if ((ret = av_frame_ref(pict, s->next_picture_ptr->f)) < 0) {
+return ret;
+}
+*got_frame = 1;
+}
 return get_consumed_bytes(s, buf_size);
+}
 
 /* skip if the header was thrashed */
 if (ret < 0) {
-- 
2.15.0.rc1.287.g2b38de12cc-goog

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

Re: [FFmpeg-devel] order T-shirts

2017-10-19 Thread Thilo Borgmann 
Hi,

> Some guys at the VDD asked for FFmpeg T-shirts.
> I'd like to do a new T-shirt order. The shirts could be given to
> multimedia devs who stop at one of our next booths.
> 
> My idea is to order 25 shirts:
> 
> 1*S
> 5*M
> 10*L
> 7*XL
> 2*XXL
> 
> Last time we ordered 5 shirts and got a price of 22,50 Euro per shirt.
> So, we are talking about a maximum price of 562,50 Euro (excluding
> delivery costs).

since I had my own shirt exchanged during VDD, I don't even have one for the 
next conf thing I might go to so I hereby revive this thread. I promised some 
shirts to Mozilla, Nicolas reminded me of getting one for him also during VDD...

I went to a nice shop today and they can offer a quite nice price for shirts 
based on Thomas' last design with breast and neck printings. So I ordered four 
samples for just 12€ each! I had the unprinted shirts in my hand and the 
quality is good in spite of that nice price. I even think that 12€ per shirt 
can't be beaten by Liu's offer to have them printed in China... shipping should 
eat up anything we might save by China-shirts.

I'll get the printed samples tomorrow and one of these will definitely find its 
way to Nicolas.
If we're happy with the samples, I'd like to order a pile of shirts for us and 
that could be the amount Thomas listed in the beginning of this thread. I'd 
also like to add some more for the shirts I've already promised to send to 
several people. This means I expect something like 350-400€ including shipping 
to various places.

This is even cheaper than what Thomas estimation was so I hope nobody objects :)

This shop can also do stitching on basecaps, with a little longer waiting time 
also the coffee mugs.
I also talked about stickers that would come in a similar manner like the nice 
ones we got from the Kodi guy some years ago and that are almost gone. However 
I want to get the shirts done first.

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

Re: [FFmpeg-devel] [PATCH] libavformat: not treat 0 as EOF

2017-10-19 Thread Nicolas George 
Le sextidi 26 vendémiaire, an CCXXVI, Michael Niedermayer a écrit :
> Seems to work, no more comments from me

I think this is correct too.

Thanks to Daniel for the efforts.

Regards,

-- 
  Nicolas George


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

Re: [FFmpeg-devel] [PATCH] lavc: drop support for OpenJPEG 1.3-2.0

2017-10-19 Thread Michael Bradshaw 
On Thu, Oct 19, 2017 at 2:28 AM, Moritz Barsnick  wrote:

> On Wed, Oct 18, 2017 at 20:21:21 -0700, Michael Bradshaw wrote:
> > I've added a note to the Changelog. Let me know if there are additional
> > notices that should be made.
>
> It was remarked that pkg-config support was introduced with *2.0.1*,
> not *2.1(.0)*. James's comment:
>

Indeed. I'm intentionally dropping support for 2.0.x.

http://ffmpeg.org/pipermail/ffmpeg-devel/2017-October/218145.html
> where his remark was:
> > OpenJPEG 2.0.1 ships a pkg-config file, so change the second argument to
> > "libopenjp2 >= 2.1.0".
>
> I consider the second line a typo. James?
>
> Indeed, this commit here adds the .pc file to openjpeg2:
> https://github.com/uclouvain/openjpeg/commit/
> 87e09a09da6493b9e25193173091fd56c2063136#diff-
> 20f723280676e973232128ea1ba322ee
> and this commit is included in the repo's "version.2.0.1" tag.
>
> Perhaps you want to relax the check (and the Changelog) - I understand
> that was James's original intent. (Not that I care for that version
> personally.)


There was an API change between 2.0 and 2.1. Since 2.1, the API has been
stable. I want to remove all the #if hacks in our code, so dropping 2.0 is
a necessary part of that.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 2/7] nutdec: Fix repeated condition

2017-10-19 Thread Mark Thompson 
On 19/10/17 00:45, Michael Niedermayer wrote:
> On Tue, Oct 17, 2017 at 11:11:54PM +0100, Mark Thompson wrote:
>> Fixes #6742.
>> ---
>>  libavformat/nutdec.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> ok, also please backport this if this issue was in releases

Sure.  It was introduced by 6621105877ce0d65724a8ab60b3a50160adbe65d, so it's 
in all supported releases (2.8 onwards).

Thanks,

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

Re: [FFmpeg-devel] [PATCH 7/9] hevc: Fix aligned array declarations

2017-10-19 Thread Mark Thompson 
On 19/10/17 00:43, Michael Niedermayer wrote:
> On Tue, Oct 17, 2017 at 10:12:23PM +0100, Mark Thompson wrote:
>> (cherry picked from commit d41e10c1485ec34aa342f7bc2e5bf4f9b6e66414)
>> ---
>> This and the following patches were found with the TI ARM compiler.
>>
>>
>>  libavcodec/hevcdsp.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> ok
> 
> also theres a
> DECLARE_ALIGNED(32, int16_t, tmp [MAX_PB_SIZE * MAX_PB_SIZE]);
> in libavcodec/hevcdec.h
> 
> which seems to have the same issue

The TI compiler doesn't seem to mind that one (single-dimensional?), but I'll 
fix it anyway in the same patch in case something else does.

A bit of grep finds some more cases in libavcodec/mips/*_mmi.c and 
libavutil/lls.h, I'll send some more patches for those later.

Thanks,

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

Re: [FFmpeg-devel] [RFC] v4l2_m2m: Fix races around freeing data on close

2017-10-19 Thread Mark Thompson 
On 19/10/17 08:28, Jorge Ramirez wrote:
> On 10/19/2017 02:10 AM, Mark Thompson wrote:
>> Refcount all of the context information.
>> ---
>> As discussed in the other thread, something like this.  We move most of the 
>> context into a refcounted buffer and AVCodecContext.priv_data is left as a 
>> stub holding a reference to it.
> 
> um, ok ... please could you send a patch so I can apply it? I see several 
> problems in v4l2_free_buffer.

What goes wrong?  It applies fine for me on current head 
(f4090940bd3024e69d236257d327f11d1e496229).

> To sum up the RFC, instead of using private_data to place the codec's 
> context, it uses private data to place a _pointer_ to an allocated codec 
> context "just" so it wont be deallocated after the codec is closed (codec 
> close frees the private_data)
> 
> Personally I think adding AV_CODEC_FLAG2_PRESERVE_PRIVDATA_ONCLOSE and use 
> private_data to hold the codec context is a cleaner/simpler approach.
> 
> - the codec requests private_data with a size (sizeof(type))
> - the code explicitly informs the API whether all memory will be released on 
> close or it will preserve it.

- All APIs in ffmpeg with this sort of private data field use them in the same 
way - they are allocated at create/alloc time (with the given size, for 
AVOptions), and freed at close/destroy time.
- Using the well-tested reference-counted buffer implementation is IMO strongly 
preferable to making ad-hoc synchronisation with atomics and semaphores.
- All other decoders use the reference-counted buffer approach (e.g. look at 
rkmpp for a direct implementation, the hwaccels all do it via hwcontext).
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [RFC] v4l2_m2m: Fix races around freeing data on close

2017-10-19 Thread Hendrik Leppkes 
On Thu, Oct 19, 2017 at 9:28 AM, Jorge Ramirez
 wrote:
> On 10/19/2017 02:10 AM, Mark Thompson wrote:
>>
>> Refcount all of the context information.
>> ---
>> As discussed in the other thread, something like this.  We move most of
>> the context into a refcounted buffer and AVCodecContext.priv_data is left as
>> a stub holding a reference to it.
>
>
> um, ok ... please could you send a patch so I can apply it? I see several
> problems in v4l2_free_buffer.
>
> To sum up the RFC, instead of using private_data to place the codec's
> context, it uses private data to place a _pointer_ to an allocated codec
> context "just" so it wont be deallocated after the codec is closed (codec
> close frees the private_data)
>
> Personally I think adding AV_CODEC_FLAG2_PRESERVE_PRIVDATA_ONCLOSE and use
> private_data to hold the codec context is a cleaner/simpler approach.
>

Using ref-counting is a mechanism we already have in-place for this
kind of thing, adding another global flag only used by one decoder is
not a preferred approach.

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

Re: [FFmpeg-devel] [PATCH] lavc: drop support for OpenJPEG 1.3-2.0

2017-10-19 Thread Moritz Barsnick 
On Wed, Oct 18, 2017 at 20:21:21 -0700, Michael Bradshaw wrote:
> I've added a note to the Changelog. Let me know if there are additional
> notices that should be made.

It was remarked that pkg-config support was introduced with *2.0.1*,
not *2.1(.0)*. James's comment:

http://ffmpeg.org/pipermail/ffmpeg-devel/2017-October/218145.html
where his remark was:
> OpenJPEG 2.0.1 ships a pkg-config file, so change the second argument to
> "libopenjp2 >= 2.1.0".

I consider the second line a typo. James?

Indeed, this commit here adds the .pc file to openjpeg2:
https://github.com/uclouvain/openjpeg/commit/87e09a09da6493b9e25193173091fd56c2063136#diff-20f723280676e973232128ea1ba322ee
and this commit is included in the repo's "version.2.0.1" tag.

Perhaps you want to relax the check (and the Changelog) - I understand
that was James's original intent. (Not that I care for that version
personally.)

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

Re: [FFmpeg-devel] [RFC] v4l2_m2m: Fix races around freeing data on close

2017-10-19 Thread Jorge Ramirez 

On 10/19/2017 02:10 AM, Mark Thompson wrote:

Refcount all of the context information.
---
As discussed in the other thread, something like this.  We move most of the 
context into a refcounted buffer and AVCodecContext.priv_data is left as a stub 
holding a reference to it.

Thoughts:
* Change is rather ugly; some structures and function arguments could probably 
be rearranged to improve it.
* Not very well tested - I'm only testing it with the decoder on s5p-mfc.  
(Encoder might well be totally broken.)
* It currently adds an extra atomic to each buffer to keep track of the 
context-references that isn't really wanted.  Cleaning up the per-plane 
references so we only go through the buffer-free sequence once would remove it.

I found several more issues while looking at this (not treated here):
* The refsync process with the semaphore is racy - it will fall over if the 
buffer unrefs are called on multiple threads at the same time.
* Buffers are requeued once for every plane they have as a consequnce of the 
per-plane references (NV12 buffer -> enqueue twice).  The later enqueues are 
ignored by the driver (QBUF returns EINVAL; look at strace), but that should 
probably still be treated as a bug.
* It seems to be able to leak all of the input packets (if refcounted?) - 
valgrind shows this, but I didn't investigate further.


about the last issue, I think this is related of how bsf is used in 3.4 
codecs and not to the v4l2 codec implementation itself
Running the v4l2 m2m decoding in 3.4 vs running 3.3 shows the following 
(I am using the flag to preserve the private_data in both cases)


ffmpeg: 3.4


[AVIOContext @ 0x590ee20] Statistics: 3997696 bytes read, 0 seeks
==2525==at 0x12A00F8: VALGRIND_PRINTF_BACKTRACE (valgrind.h:6303)
==2525==by 0x12A0DD3: av_log_default_callback (log.c:355)
==2525==by 0x21CD03: log_callback_report (cmdutils.c:110)
==2525==by 0x12A0FAF: av_vlog (log.c:383)
==2525==by 0x12A0F37: av_log (log.c:375)
==2525==by 0x224287: term_exit (ffmpeg.c:317)
==2525==by 0x224E0B: ffmpeg_cleanup (ffmpeg.c:618)
==2525==by 0x21CDFB: exit_program (cmdutils.c:138)
==2525==by 0x233F03: main (ffmpeg.c:4824)
==2525==
==2525== HEAP SUMMARY:
==2525== in use at exit: 4,015,310 bytes in 617 blocks
==2525==   total heap usage: 16,338 allocs, 15,721 frees, 21,262,380 
bytes allocated

==2525==
==2525== 18,156 bytes in 1 blocks are possibly lost in loss record 5 of 8
==2525==at 0x4844C38: malloc (vg_replace_malloc.c:298)
==2525==by 0x4847197: realloc (vg_replace_malloc.c:785)
==2525==by 0x12A4773: av_realloc (mem.c:144)
==2525==by 0x128FABB: av_buffer_realloc (buffer.c:177)
==2525==by 0x67F49F: packet_alloc (avpacket.c:77)
==2525==by 0x68118F: av_packet_ref (avpacket.c:636)
==2525==by 0x6078EB: add_to_pktbuf (utils.c:435)
==2525==by 0x60B483: parse_packet (utils.c:1470)
==2525==by 0x60BBE3: read_frame_internal (utils.c:1613)
==2525==by 0x60C22B: av_read_frame (utils.c:1724)
==2525==by 0x2317AF: get_input_packet (ffmpeg.c:4097)
==2525==by 0x231CAB: process_input (ffmpeg.c:4217)
==2525==
==2525== 3,996,554 (4,920 direct, 3,991,634 indirect) bytes in 205 
blocks are definitely lost in loss record 8 of 8

==2525==at 0x4847248: memalign (vg_replace_malloc.c:857)
==2525==by 0x484735B: posix_memalign (vg_replace_malloc.c:1020)
==2525==by 0x12A46D3: av_malloc (mem.c:87)
==2525==by 0x12A49B3: av_mallocz (mem.c:224)
==2525==by 0x128F7DF: av_buffer_ref (buffer.c:95)
==2525==by 0x6811FF: av_packet_ref (avpacket.c:644)
==2525==by 0x6CCEF3: avcodec_send_packet (decode.c:666)
==2525==by 0x22B043: decode (ffmpeg.c:2265)
==2525==by 0x22B7BF: decode_video (ffmpeg.c:2409)
==2525==by 0x22C573: process_input_packet (ffmpeg.c:2650)
==2525==by 0x232F93: process_input (ffmpeg.c:4442)
==2525==by 0x23348F: transcode_step (ffmpeg.c:4553)
==2525==
==2525== LEAK SUMMARY:
==2525==definitely lost: 4,920 bytes in 205 blocks
==2525==indirectly lost: 3,991,634 bytes in 409 blocks
==2525==  possibly lost: 18,156 bytes in 1 blocks
==2525==still reachable: 600 bytes in 2 blocks
==2525== suppressed: 0 bytes in 0 blocks
==2525== Reachable blocks (those to which a pointer was found) are not 
shown.

==2525== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==2525==
==2525== For counts of detected and suppressed errors, rerun with: -v
==2525== Use --track-origins=yes to see where uninitialised values come 
from

==2525== ERROR SUMMARY: 3 errors from 3 contexts (suppressed: 0 from 0)


ffmpeg 3.3
==

[AVIOContext @ 0x590ee20] Statistics: 2621440 bytes read, 0 seeks
==11047==at 0x1246920: VALGRIND_PRINTF_BACKTRACE (valgrind.h:6303)
==11047==by 0x12475FB: av_log_default_callback (log.c:355)
==11047==by 0x203BEB: log_callback_report (cmdutils.c:110)
==11047==by 0x12477D7: av_vlog (log.c:383)
==11047==by 0x124775F: av_log (log.c:375)

Re: [FFmpeg-devel] [PATCH 4/4] lavf/movdec: request probing for an ambiguous codec tag

2017-10-19 Thread Jan Ekstrom 
On Thu, Oct 19, 2017 at 10:39 AM, Rodger Combs  wrote:
> ---
>  libavformat/isom.c | 2 ++
>  1 file changed, 2 insertions(+)

This change makes sense due to the ambiguousness of the container mapping. LGTM.

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

Re: [FFmpeg-devel] [PATCH 3/4] lavf/utils: add MP2 to the probing list

2017-10-19 Thread Jan Ekstrom 
On Thu, Oct 19, 2017 at 10:39 AM, Rodger Combs  wrote:
> ---

Seems like a valid change to enable the following change of enabling
probing in movdec for mp2/3 in ISOBMFF.

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

[FFmpeg-devel] [PATCH 2/4] lavf: identify MP1 and MP2 as distinct containers from MP3

2017-10-19 Thread Rodger Combs 
This allows us to report the correct codec ID here
---
 libavformat/Makefile |  2 ++
 libavformat/allformats.c |  4 +--
 libavformat/mp3dec.c | 66 +++-
 libavformat/utils.c  |  3 +--
 libavformat/version.h|  4 +--
 5 files changed, 50 insertions(+), 29 deletions(-)

diff --git a/libavformat/Makefile b/libavformat/Makefile
index 2522a3e768..a978482211 100644
--- a/libavformat/Makefile
+++ b/libavformat/Makefile
@@ -296,7 +296,9 @@ OBJS-$(CONFIG_MOV_DEMUXER)   += mov.o 
mov_chan.o replaygain.o
 OBJS-$(CONFIG_MOV_MUXER) += movenc.o avc.o hevc.o vpcc.o \
 movenchint.o mov_chan.o rtp.o \
 movenccenc.o rawutils.o
+OBJS-$(CONFIG_MP1_DEMUXER)   += mp3dec.o replaygain.o
 OBJS-$(CONFIG_MP1_MUXER) += rawenc.o
+OBJS-$(CONFIG_MP2_DEMUXER)   += mp3dec.o replaygain.o
 OBJS-$(CONFIG_MP2_MUXER) += rawenc.o
 OBJS-$(CONFIG_MP3_DEMUXER)   += mp3dec.o replaygain.o
 OBJS-$(CONFIG_MP3_MUXER) += mp3enc.o rawenc.o id3v2enc.o
diff --git a/libavformat/allformats.c b/libavformat/allformats.c
index 32e9a979bc..c02cdacb7b 100644
--- a/libavformat/allformats.c
+++ b/libavformat/allformats.c
@@ -189,8 +189,8 @@ static void register_all(void)
 REGISTER_DEMUXER (MM,   mm);
 REGISTER_MUXDEMUX(MMF,  mmf);
 REGISTER_MUXDEMUX(MOV,  mov);
-REGISTER_MUXER   (MP1,  mp1);
-REGISTER_MUXER   (MP2,  mp2);
+REGISTER_MUXDEMUX(MP1,  mp1);
+REGISTER_MUXDEMUX(MP2,  mp2);
 REGISTER_MUXDEMUX(MP3,  mp3);
 REGISTER_MUXER   (MP4,  mp4);
 REGISTER_DEMUXER (MPC,  mpc);
diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
index a5c4f2ea12..d405a98d7b 100644
--- a/libavformat/mp3dec.c
+++ b/libavformat/mp3dec.c
@@ -66,7 +66,7 @@ static int check(AVIOContext *pb, int64_t pos, uint32_t 
*header);
 
 /* mp3 read */
 
-static int mp3_read_probe(AVProbeData *p)
+static int mpa_read_probe(AVProbeData *p, int layer)
 {
 int max_frames, first_frames = 0;
 int whole_used = 0;
@@ -89,7 +89,7 @@ static int mp3_read_probe(AVProbeData *p)
 
 header = AV_RB32(buf2);
 ret = avpriv_mpegaudio_decode_header(, header);
-if (ret != 0)
+if (ret != 0 || h.layer != layer)
 break;
 buf2 += h.frame_size;
 }
@@ -105,7 +105,8 @@ static int mp3_read_probe(AVProbeData *p)
 if   (first_frames>=7) return AVPROBE_SCORE_EXTENSION + 1;
 else if(max_frames>200)return AVPROBE_SCORE_EXTENSION;
 else if(max_frames>=4 && max_frames >= p->buf_size/1) return 
AVPROBE_SCORE_EXTENSION / 2;
-else if(ff_id3v2_match(buf0, ID3v2_DEFAULT_MAGIC) && 
2*ff_id3v2_tag_len(buf0) >= p->buf_size)
+else if(ff_id3v2_match(buf0, ID3v2_DEFAULT_MAGIC) && 
2*ff_id3v2_tag_len(buf0) >= p->buf_size &&
+(p->buf_size < PROBE_BUF_MAX || layer == 3))
return p->buf_size < PROBE_BUF_MAX ? 
AVPROBE_SCORE_EXTENSION / 4 : AVPROBE_SCORE_EXTENSION - 2;
 else if(first_frames > 1 && whole_used) return 5;
 else if(max_frames>=1 && max_frames >= p->buf_size/1) return 1;
@@ -113,6 +114,12 @@ static int mp3_read_probe(AVProbeData *p)
 //mpegps_mp3_unrecognized_format.mpg has max_frames=3
 }
 
+#define READ_PROBE(l) \
+static int mp##l##_read_probe(AVProbeData *p) \
+{ \
+return mpa_read_probe(p, l); \
+}
+
 static void read_xing_toc(AVFormatContext *s, int64_t filesize, int64_t 
duration)
 {
 int i;
@@ -341,7 +348,7 @@ static int mp3_parse_vbr_tags(AVFormatContext *s, AVStream 
*st, int64_t base)
 return 0;
 }
 
-static int mp3_read_header(AVFormatContext *s)
+static int mpa_read_header(AVFormatContext *s, enum AVCodecID id)
 {
 MP3DecContext *mp3 = s->priv_data;
 AVStream *st;
@@ -357,7 +364,7 @@ static int mp3_read_header(AVFormatContext *s)
 return AVERROR(ENOMEM);
 
 st->codecpar->codec_type = AVMEDIA_TYPE_AUDIO;
-st->codecpar->codec_id = AV_CODEC_ID_MP3;
+st->codecpar->codec_id = id;
 st->need_parsing = AVSTREAM_PARSE_FULL_RAW;
 st->start_time = 0;
 
@@ -422,6 +429,12 @@ static int mp3_read_header(AVFormatContext *s)
 return 0;
 }
 
+#define READ_HEADER(l) \
+static int mp##l##_read_header(AVFormatContext *s) \
+{ \
+return mpa_read_header(s, AV_CODEC_ID_MP##l); \
+}
+
 #define MP3_PACKET_SIZE 1024
 
 static int mp3_read_packet(AVFormatContext *s, AVPacket *pkt)
@@ -582,23 +595,30 @@ static const AVOption options[] = {
 { NULL },
 };
 
-static const AVClass demuxer_class = {
-.class_name = "mp3",
-.item_name  = av_default_item_name,
-.option = options,
-.version= LIBAVUTIL_VERSION_INT,
-.category   = AV_CLASS_CATEGORY_DEMUXER,
+#define DECLARE_LAYER(l, ext) \

[FFmpeg-devel] [PATCH 4/4] lavf/movdec: request probing for an ambiguous codec tag

2017-10-19 Thread Rodger Combs 
---
 libavformat/isom.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libavformat/isom.c b/libavformat/isom.c
index 77983c5eaa..8b3f88ce74 100644
--- a/libavformat/isom.c
+++ b/libavformat/isom.c
@@ -519,6 +519,8 @@ FF_ENABLE_DEPRECATION_WARNINGS
 codec_id= ff_codec_get_id(ff_mp4_obj_type, object_type_id);
 if (codec_id)
 st->codecpar->codec_id = codec_id;
+if (object_type_id == 0x6B) // This can be either MP3 or MP2; let 
probe_codec decide
+st->request_probe = 5;
 av_log(fc, AV_LOG_TRACE, "esds object type id 0x%02x\n", object_type_id);
 len = ff_mp4_read_descr(fc, pb, );
 if (tag == MP4DecSpecificDescrTag) {
-- 
2.14.1

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

[FFmpeg-devel] [PATCH 1/4] lavf: add MP1 muxer

2017-10-19 Thread Rodger Combs 
---
 libavformat/Makefile |  1 +
 libavformat/allformats.c |  1 +
 libavformat/rawenc.c | 13 +
 3 files changed, 15 insertions(+)

diff --git a/libavformat/Makefile b/libavformat/Makefile
index d955a8b12a..2522a3e768 100644
--- a/libavformat/Makefile
+++ b/libavformat/Makefile
@@ -296,6 +296,7 @@ OBJS-$(CONFIG_MOV_DEMUXER)   += mov.o 
mov_chan.o replaygain.o
 OBJS-$(CONFIG_MOV_MUXER) += movenc.o avc.o hevc.o vpcc.o \
 movenchint.o mov_chan.o rtp.o \
 movenccenc.o rawutils.o
+OBJS-$(CONFIG_MP1_MUXER) += rawenc.o
 OBJS-$(CONFIG_MP2_MUXER) += rawenc.o
 OBJS-$(CONFIG_MP3_DEMUXER)   += mp3dec.o replaygain.o
 OBJS-$(CONFIG_MP3_MUXER) += mp3enc.o rawenc.o id3v2enc.o
diff --git a/libavformat/allformats.c b/libavformat/allformats.c
index 405ddb5ad9..32e9a979bc 100644
--- a/libavformat/allformats.c
+++ b/libavformat/allformats.c
@@ -189,6 +189,7 @@ static void register_all(void)
 REGISTER_DEMUXER (MM,   mm);
 REGISTER_MUXDEMUX(MMF,  mmf);
 REGISTER_MUXDEMUX(MOV,  mov);
+REGISTER_MUXER   (MP1,  mp1);
 REGISTER_MUXER   (MP2,  mp2);
 REGISTER_MUXDEMUX(MP3,  mp3);
 REGISTER_MUXER   (MP4,  mp4);
diff --git a/libavformat/rawenc.c b/libavformat/rawenc.c
index f640121cb4..1a30ba6c05 100644
--- a/libavformat/rawenc.c
+++ b/libavformat/rawenc.c
@@ -361,6 +361,19 @@ AVOutputFormat ff_mlp_muxer = {
 };
 #endif
 
+#if CONFIG_MP1_MUXER
+AVOutputFormat ff_mp1_muxer = {
+.name  = "mp1",
+.long_name = NULL_IF_CONFIG_SMALL("MP1 (MPEG audio layer 1)"),
+.mime_type = "audio/mpeg",
+.extensions= "mp1,m1a",
+.audio_codec   = AV_CODEC_ID_MP1,
+.video_codec   = AV_CODEC_ID_NONE,
+.write_packet  = ff_raw_write_packet,
+.flags = AVFMT_NOTIMESTAMPS,
+};
+#endif
+
 #if CONFIG_MP2_MUXER
 AVOutputFormat ff_mp2_muxer = {
 .name  = "mp2",
-- 
2.14.1

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

Re: [FFmpeg-devel] [PATCH] avcodec/v4l2: fix access to priv_data after codec close.

2017-10-19 Thread Jorge Ramirez 

On 10/18/2017 09:40 PM, Michael Niedermayer wrote:

On Wed, Oct 18, 2017 at 09:46:40AM +0200, Jorge Ramirez wrote:

On 10/18/2017 12:34 AM, Mark Thompson wrote:

  int ff_v4l2_m2m_codec_end(AVCodecContext *avctx)
  {
-V4L2m2mContext* s = avctx->priv_data;
-int ret;
+V4L2m2mContext *m2m, *s = avctx->priv_data;
+int i, ret;
  ret = ff_v4l2_context_set_status(>output, VIDIOC_STREAMOFF);
  if (ret)
@@ -325,12 +325,31 @@ int ff_v4l2_m2m_codec_end(AVCodecContext *avctx)
  av_log(avctx, AV_LOG_ERROR, "VIDIOC_STREAMOFF %s\n", s->capture.name);
  ff_v4l2_context_release(>output);
+sem_destroy(>refsync);
-if (atomic_load(>refcount))
-av_log(avctx, AV_LOG_ERROR, "ff_v4l2m2m_codec_end leaving pending 
buffers\n");
+if (atomic_load(>refcount)) {
+av_log(avctx, AV_LOG_DEBUG, "ff_v4l2m2m_codec_end, referenced buffers %d\n", 
atomic_load(>refcount));
+
+/* We are about to free the private data while the user still has 
references to the buffers.
+ * This means that when the user releases those buffers, the 
v4l2_free_buffer callback will be pointing to free'd memory.
+ * Duplicate the m2m context and update the buffers.
+ */
+m2m = av_mallocz(sizeof(*m2m));
+if (m2m) {
+memcpy(m2m, s, sizeof(V4L2m2mContext));
+for (i = 0; i < s->capture.num_buffers; i++)
+s->capture.buffers[i].m2m = m2m;
+
+return 0;
+}

No.

The user can call av_buffer_unref() and therefore v4l2_free_buffer() 
asynchronously on another thread while this manoeuvre is in progress.


right. as a matter of fact, this synchronization can not be done
using private data (the opaque structure passed to the callback must
contain all the required information (and updated!) at any time). On
top of this being hard to implement would complicate the current
code quite a bit making it also very hard to maintain. So agreed.

So back to a previous proposal, would the flag below be acceptable? [1]

Seems sensible to me to give responsibility of the private data
deallocation to the codec itself independently of the close function
since the buffers allocated by the codec controlled by a separate
thread will need access to that data.

Put differently since the close function can not be made responsible
for deallocating mmap/queued memory in V4L2 (or even closing the
kernel driver) when the user keeps buffers, it should not be allowed
to delete the codec private data since in my view it forces the
design to take a lot of unnecessary actions for no real reason.

With the proposal below, the code remains cleaner and free of race
conditions.

[1] ..

diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index 52cc5b0..0a3ca0c 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -982,6 +982,11 @@ typedef struct RcOverride{
   * Do not reset ASS ReadOrder field on flush (subtitles decoding)
   */
  #define AV_CODEC_FLAG2_RO_FLUSH_NOOP  (1 << 30)
+/**
+ * Closing the codec doesnt release the context priv_data (it
becomes the codec
+ * responsibility)
+ */
+#define AV_CODEC_FLAG2_PRESERVE_PRIVDATA_ONCLOSE  (1 << 31)

  /* Unsupported options :
   *  Syntax Arithmetic coding (SAC)
diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index 9551f31..b6c5adf 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -1211,7 +1211,9 @@ av_cold int avcodec_close(AVCodecContext *avctx)
  if (avctx->priv_data && avctx->codec && avctx->codec->priv_class)
  av_opt_free(avctx->priv_data);
  av_opt_free(avctx);
-av_freep(>priv_data);
+if (!(avctx->flags2 & AV_CODEC_FLAG2_PRESERVE_PRIVDATA_ONCLOSE))
+av_freep(>priv_data);


This may be solving a similar issue as our udp.c cleanup with its
thread. Not sure, i didnt follow this discussion closely.
just wanted to mention as one migt benefit from the other if its the
same issue.

Another thing that came to my mind, if the context (and other resources)
are deallocated with a delay beyond closing. Is this still correct if
the process terminates before or in the middle.
Or is that preented somehow ?


sorry not sure I follow, since the buffer references also belong to the 
process closing the process would end up closing the file descriptor (ie 
v4l2 driver).
the driver would have to do its internal house keeping (release its 
queued buffers and so on).

I dont think this would be an issue.


There would be at least spurious warnings from memory debuggers if
deallocation isnt run at all


but the change above is not functionally different to what we do today: 
just gives the codec more control over its private_data




[...]


___
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