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(&s->refcount, 1, memory_order_acq_rel);
- if (s->reinit) {
- if (!atomic_load(&s->refcount))
- sem_post(&s->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(&avbuf->context_refcount, 1) == 1) {
+ atomic_fetch_sub_explicit(&s->refcount, 1,
memory_order_acq_rel);
+
+ if (s->reinit) {
+ if (!atomic_load(&s->refcount))
+ sem_post(&s->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(&avbuf->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 (320716800 bytes) demuxed
Output file #0 (out/out.h264.mp4):
Output stream #0:0 (video): 232 frames encoded; 232 packets muxed
(563494 bytes);
Total: 232 packets (563494 bytes) muxed
232 frames successfully decoded, 0 decoding errors
[AVIOContext @ 0x5aa9270] Statistics: 2 seeks, 6 writeouts
[AVIOContext @ 0x59103b0] Statistics: 320716800 bytes read, 0 seeks
==10241== at 0x129F8B8: VALGRIND_PRINTF_BACKTRACE (valgrind.h:6303)
==10241== by 0x12A0593: av_log_default_callback (log.c:355)
==10241== by 0x21CEC3: log_callback_report (cmdutils.c:110)
==10241== by 0x12A076F: av_vlog (log.c:383)
==10241== by 0x12A06F7: av_log (log.c:375)
==10241== by 0x224447: term_exit (ffmpeg.c:317)
==10241== by 0x224FCB: ffmpeg_cleanup (ffmpeg.c:618)
==10241== by 0x21CFBB: exit_program (cmdutils.c:138)
==10241== by 0x23412B: main (ffmpeg.c:4832)
==10241==
==10241== HEAP SUMMARY:
==10241== in use at exit: 600 bytes in 2 blocks
==10241== total heap usage: 4,323 allocs, 4,321 frees, 325,278,370
bytes allocated
==10241==
==10241== LEAK SUMMARY:
==10241== definitely lost: 0 bytes in 0 blocks
==10241== indirectly lost: 0 bytes in 0 blocks
==10241== possibly lost: 0 bytes in 0 blocks
==10241== still reachable: 600 bytes in 2 blocks
==10241== suppressed: 0 bytes in 0 blocks
==10241== Reachable blocks (those to which a pointer was found) are not
shown.
==10241== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==10241==
==10241== For counts of detected and suppressed errors, rerun with: -v
==10241== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
