Re: [FFmpeg-devel] [PATCH] videotoolbox: allow to enable the async decoding.

2015-10-06 Thread Clément Bœsch 
On Thu, Sep 24, 2015 at 11:45:42AM +0200, Clément Bœsch wrote:
> On Sun, Aug 09, 2015 at 01:11:44PM +0200, Sebastien Zwickert wrote:
> > This patch allows to use the Videotoolbox API in asynchonous mode.
> > Note that when using async decoding the user is responsible for
> > releasing the async frame.
> > Moreover, an option called videotoolbox_async was added to enable
> > async decoding with ffmpeg CLI.
> > 
> > ---
> >  ffmpeg.h  |   1 +
> >  ffmpeg_opt.c  |   1 +
> >  ffmpeg_videotoolbox.c |  69 +
> >  libavcodec/videotoolbox.c | 186 
> > --
> >  libavcodec/videotoolbox.h |  73 ++
> >  5 files changed, 294 insertions(+), 36 deletions(-)
> > 
> 
> Ping and more comments on this:
> 
> - is it meaningful to have a limit on the internal queue size (for memory
>   load for instance), or it's not relevant in case of hw accel?
> 

To answer myself: a limit is actually required or it causes a crash at
least on iOS after about 150 frames in the queue.

Following is a hack I needed:

diff --git a/libavcodec/videotoolbox.c b/libavcodec/videotoolbox.c
index 9664f16..73b9023 100644
--- a/libavcodec/videotoolbox.c
+++ b/libavcodec/videotoolbox.c
@@ -222,6 +222,8 @@ static void videotoolbox_clear_queue(struct 
AVVideotoolboxContext *videotoolbox)
 av_videotoolbox_release_async_frame(top_frame);
 }
 
+videotoolbox->nb_frames = 0;
+
 videotoolbox_lock_operation(>queue_mutex, AV_LOCK_RELEASE);
 }
 
@@ -379,6 +381,9 @@ static void videotoolbox_decoder_callback(void *opaque,
 
 videotoolbox_lock_operation(>queue_mutex, 
AV_LOCK_OBTAIN);
 
+while (videotoolbox->nb_frames == 16)
+pthread_cond_wait(>queue_reduce, 
videotoolbox->queue_mutex);
+
 queue_walker = videotoolbox->queue;
 
 if (!queue_walker || (new_frame->pts < queue_walker->pts)) {
@@ -401,6 +406,7 @@ static void videotoolbox_decoder_callback(void *opaque,
 }
 }
 
+videotoolbox->nb_frames++;
 videotoolbox_lock_operation(>queue_mutex, 
AV_LOCK_RELEASE);
 }
 }
@@ -617,6 +623,8 @@ static int videotoolbox_default_init(AVCodecContext *avctx)
 return -1;
 
 videotoolbox_lock_operation(>queue_mutex, 
AV_LOCK_CREATE);
+
+pthread_cond_init(>queue_reduce, NULL);
 }
 
 switch( avctx->codec_id ) {
@@ -700,6 +708,8 @@ static void videotoolbox_default_free(AVCodecContext *avctx)
 
 videotoolbox_clear_queue(videotoolbox);
 
+pthread_cond_destroy(>queue_reduce);
+
 if (videotoolbox->queue_mutex != NULL)
 videotoolbox_lock_operation(>queue_mutex, 
AV_LOCK_DESTROY);
 }
@@ -826,8 +836,11 @@ AVVideotoolboxAsyncFrame 
*av_videotoolbox_pop_async_frame(AVVideotoolboxContext
 videotoolbox_lock_operation(>queue_mutex, AV_LOCK_OBTAIN);
 top_frame = videotoolbox->queue;
 videotoolbox->queue = top_frame->next_frame;
+videotoolbox->nb_frames--;
 videotoolbox_lock_operation(>queue_mutex, AV_LOCK_RELEASE);
 
+pthread_cond_signal(>queue_reduce);
+
 return top_frame;
 }
 
diff --git a/libavcodec/videotoolbox.h b/libavcodec/videotoolbox.h
index b5bf030..db41b4a 100644
--- a/libavcodec/videotoolbox.h
+++ b/libavcodec/videotoolbox.h
@@ -113,6 +113,9 @@ typedef struct AVVideotoolboxContext {
  */
 void *queue_mutex;
 
+pthread_cond_t queue_reduce;
+int nb_frames;
+
 } AVVideotoolboxContext;
 
 /**


> - isn't it missing a flush mechanism so seeking can be accurate?

videotoolbox_clear_queue() should probably be exported

[...]

But again all of this is more in the spirit of a proper async API...

BTW, I can confirm it fixes the playback speed on these devices.

-- 
Clément B.


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

Re: [FFmpeg-devel] [PATCH] videotoolbox: allow to enable the async decoding.

2015-09-24 Thread Clément Bœsch 
On Sun, Aug 09, 2015 at 01:11:44PM +0200, Sebastien Zwickert wrote:
> This patch allows to use the Videotoolbox API in asynchonous mode.
> Note that when using async decoding the user is responsible for
> releasing the async frame.
> Moreover, an option called videotoolbox_async was added to enable
> async decoding with ffmpeg CLI.
> 
> ---
>  ffmpeg.h  |   1 +
>  ffmpeg_opt.c  |   1 +
>  ffmpeg_videotoolbox.c |  69 +
>  libavcodec/videotoolbox.c | 186 
> --
>  libavcodec/videotoolbox.h |  73 ++
>  5 files changed, 294 insertions(+), 36 deletions(-)
> 

Ping and more comments on this:

- is it meaningful to have a limit on the internal queue size (for memory
  load for instance), or it's not relevant in case of hw accel?

- isn't it missing a flush mechanism so seeking can be accurate?

[...]

-- 
Clément B.


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

Re: [FFmpeg-devel] [PATCH] videotoolbox: allow to enable the async decoding.

2015-08-15 Thread Clément Bœsch 
On Sun, Aug 09, 2015 at 01:19:04PM +0200, Hendrik Leppkes wrote:
[...]
 Isnt there a way to put a handle to the async frame into the AVFrame
 somehow, which can then be used by the user code to get it from VT?
 

Yes that would be great; the special AVVideotoolboxAsyncFrame is kind of
annoying to deal with if you're relying on the AVFrame ref counting all
around in your app.

I would like to see av_videotoolbox_pop_async_frame poping an AVFrame (or
NULL if nothing available yet?) with the CVPixelBuffer in data[3] (just
like with sync mode), assuming that's do-able.

[...]

-- 
Clément B.


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

Re: [FFmpeg-devel] [PATCH] videotoolbox: allow to enable the async decoding.

2015-08-13 Thread Zhang Rui 
2015-08-13 14:26 GMT+08:00 Clément Bœsch u...@pkh.me:
 On Thu, Aug 13, 2015 at 01:07:38PM +0800, Zhang Rui wrote:
   status = VTDecompressionSessionDecodeFrame(videotoolbox-session,
  sample_buf,
  -   0,   // decodeFlags
  +   decodeFlags,
  NULL,// 
  sourceFrameRefCon
  0);  // infoFlagsOut

 Better keep tracking how many decoding samples,
 that are passed in and not yet received in callback.

 VideoToolbox doesn't limit the number of frames in callback thread in
 async mode.

 doc used to say that VTDecompressionSessionDecodeFrame will block when the
 internal queue is full in async mode. I haven't check if this is verified
 in practice though.

I was wrong. After a real test, I found out that
VTDecompressionSessionDecodeFrame does block when
there are several frames in VideoToolbox's internal queue.

But when I enter background by pressing home button while async-decoding.
VTB hangs for a while and returns kVTVideoDecoderMalfunctionErr.
So I have to track the number of frames in VTB's queue,
and limit it to 3 frames as a workaround in my own player.

I guess this problem may related to the app state.
But neither doc nor demo says anything about it.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] videotoolbox: allow to enable the async decoding.

2015-08-13 Thread Clément Bœsch 
On Thu, Aug 13, 2015 at 01:07:38PM +0800, Zhang Rui wrote:
   status = VTDecompressionSessionDecodeFrame(videotoolbox-session,
  sample_buf,
  -   0,   // decodeFlags
  +   decodeFlags,
  NULL,// 
  sourceFrameRefCon
  0);  // infoFlagsOut
 
 Better keep tracking how many decoding samples,
 that are passed in and not yet received in callback.
 
 VideoToolbox doesn't limit the number of frames in callback thread in
 async mode.

doc used to say that VTDecompressionSessionDecodeFrame will block when the
internal queue is full in async mode. I haven't check if this is verified
in practice though.

[...]

-- 
Clément B.


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

Re: [FFmpeg-devel] [PATCH] videotoolbox: allow to enable the async decoding.

2015-08-12 Thread Zhang Rui 
  status = VTDecompressionSessionDecodeFrame(videotoolbox-session,
 sample_buf,
 -   0,   // decodeFlags
 +   decodeFlags,
 NULL,// sourceFrameRefCon
 0);  // infoFlagsOut

Better keep tracking how many decoding samples,
that are passed in and not yet received in callback.

VideoToolbox doesn't limit the number of frames in callback thread in
async mode.
It could be out of memory soon on iPhone without limit, and sooner
when application is in background.

 @@ -586,6 +686,15 @@ static void videotoolbox_default_free(AVCodecContext 
 *avctx)
 if (videotoolbox-cm_fmt_desc)
 CFRelease(videotoolbox-cm_fmt_desc);

 +if (videotoolbox-useAsyncDecoding) {
 +
 VTDecompressionSessionWaitForAsynchronousFrames(videotoolbox-session);

I doubt if VTDecompressionSessionWaitForAsynchronousFrames guarantee
there are no more callback. Doc only says all samples are emitted.

Better wait all samples received in callback,
or add some check in callback to disable further access to context.

 +
 +videotoolbox_clear_queue(videotoolbox);
 +
 +if (videotoolbox-queue_mutex != NULL)
 +videotoolbox_lock_operation(videotoolbox-queue_mutex, 
 AV_LOCK_DESTROY);
 +}
 +
 if (videotoolbox-session)
 VTDecompressionSessionInvalidate(videotoolbox-session);
 }
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] videotoolbox: allow to enable the async decoding.

2015-08-12 Thread Zhang Rui 
2015-08-09 19:19 GMT+08:00 Hendrik Leppkes h.lepp...@gmail.com:
 On Sun, Aug 9, 2015 at 1:11 PM, Sebastien Zwickert dilar...@gmail.com wrote:
 This patch allows to use the Videotoolbox API in asynchonous mode.
 Note that when using async decoding the user is responsible for
 releasing the async frame.
 Moreover, an option called videotoolbox_async was added to enable
 async decoding with ffmpeg CLI.


 I'm not sure about your approach. I can already see you getting loads
 of problems with frame order. pts is not guaranteed to be present or
 reliable, and using it for re-ordering your async frames is probably
 unreliable at best.
 avcodec does proper re-ordering, and the only reliable way would be to
 somehow manage to associate the AVFrame avcodec is going to output
 with your async frame, so that you can trust avcodecs re-ordering.

There is a trick used in VLC and XBMC.

if (newFrame-pts != AV_NOPTS_VALUE) {
newFrame-sort = newFrame-pts;
} else {
newFrame-sort = newFrame-dts;
}

avcodec API is better if any exists

 Isnt there a way to put a handle to the async frame into the AVFrame
 somehow, which can then be used by the user code to get it from VT?

The fourth parameter ''sourceFrameRefCon of VTDecompressionSessionDecodeFrame
 is passed to callback.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] videotoolbox: allow to enable the async decoding.

2015-08-09 Thread Sebastien Zwickert 
This patch allows to use the Videotoolbox API in asynchonous mode.
Note that when using async decoding the user is responsible for
releasing the async frame.
Moreover, an option called videotoolbox_async was added to enable
async decoding with ffmpeg CLI.

---
 ffmpeg.h  |   1 +
 ffmpeg_opt.c  |   1 +
 ffmpeg_videotoolbox.c |  69 +
 libavcodec/videotoolbox.c | 186 --
 libavcodec/videotoolbox.h |  73 ++
 5 files changed, 294 insertions(+), 36 deletions(-)

diff --git a/ffmpeg.h b/ffmpeg.h
index 6544e6f..73a1031 100644
--- a/ffmpeg.h
+++ b/ffmpeg.h
@@ -522,6 +522,7 @@ extern AVIOContext *progress_avio;
 extern float max_error_rate;
 extern int vdpau_api_ver;
 extern char *videotoolbox_pixfmt;
+extern int videotoolbox_async;
 
 extern const AVIOInterruptCB int_cb;
 
diff --git a/ffmpeg_opt.c b/ffmpeg_opt.c
index 28d3051..91be9b9 100644
--- a/ffmpeg_opt.c
+++ b/ffmpeg_opt.c
@@ -3238,6 +3238,7 @@ const OptionDef options[] = {
 #endif
 #if CONFIG_VDA || CONFIG_VIDEOTOOLBOX
 { videotoolbox_pixfmt, HAS_ARG | OPT_STRING | OPT_EXPERT, { 
videotoolbox_pixfmt},  },
+{ videotoolbox_async, HAS_ARG | OPT_INT | OPT_EXPERT, { 
videotoolbox_async},  },
 #endif
 { autorotate,   HAS_ARG | OPT_BOOL | OPT_SPEC |
   OPT_EXPERT | OPT_INPUT,  
  { .off = OFFSET(autorotate) },
diff --git a/ffmpeg_videotoolbox.c b/ffmpeg_videotoolbox.c
index 6688452..0bb0600 100644
--- a/ffmpeg_videotoolbox.c
+++ b/ffmpeg_videotoolbox.c
@@ -34,21 +34,42 @@ typedef struct VTContext {
 } VTContext;
 
 char *videotoolbox_pixfmt;
+int videotoolbox_async;
 
 static int videotoolbox_retrieve_data(AVCodecContext *s, AVFrame *frame)
 {
 InputStream *ist = s-opaque;
 VTContext  *vt = ist-hwaccel_ctx;
-CVPixelBufferRef pixbuf = (CVPixelBufferRef)frame-data[3];
-OSType pixel_format = CVPixelBufferGetPixelFormatType(pixbuf);
+AVVideotoolboxContext *videotoolbox = s-hwaccel_context;
+AVVideotoolboxAsyncFrame *async_frame = NULL;
+CVPixelBufferRef pixbuf;
+OSType pixel_format;
 CVReturn err;
 uint8_t *data[4] = { 0 };
 int linesize[4] = { 0 };
 int planes, ret, i;
 char codec_str[32];
+int width, height;
 
 av_frame_unref(vt-tmp_frame);
 
+if (videotoolbox-useAsyncDecoding) {
+async_frame = av_videotoolbox_pop_async_frame(videotoolbox);
+
+if (!async_frame)
+return -1;
+
+pixbuf = async_frame-cv_buffer;
+width  = CVPixelBufferGetWidth(pixbuf);
+height = CVPixelBufferGetHeight(pixbuf);
+} else {
+pixbuf = (CVPixelBufferRef)frame-data[3];
+width  = frame-width;
+height = frame-height;
+}
+
+pixel_format = CVPixelBufferGetPixelFormatType(pixbuf);
+
 switch (pixel_format) {
 case kCVPixelFormatType_420YpCbCr8Planar: vt-tmp_frame-format = 
AV_PIX_FMT_YUV420P; break;
 case kCVPixelFormatType_422YpCbCr8:   vt-tmp_frame-format = 
AV_PIX_FMT_UYVY422; break;
@@ -60,19 +81,21 @@ static int videotoolbox_retrieve_data(AVCodecContext *s, 
AVFrame *frame)
 av_get_codec_tag_string(codec_str, sizeof(codec_str), s-codec_tag);
 av_log(NULL, AV_LOG_ERROR,
%s: Unsupported pixel format: %s\n, codec_str, 
videotoolbox_pixfmt);
-return AVERROR(ENOSYS);
+ret = AVERROR(ENOSYS);
+goto fail;
 }
 
-vt-tmp_frame-width  = frame-width;
-vt-tmp_frame-height = frame-height;
+vt-tmp_frame-width  = width;
+vt-tmp_frame-height = height;
 ret = av_frame_get_buffer(vt-tmp_frame, 32);
-if (ret  0)
-return ret;
-
+if (ret  0) {
+goto fail;
+}
 err = CVPixelBufferLockBaseAddress(pixbuf, kCVPixelBufferLock_ReadOnly);
 if (err != kCVReturnSuccess) {
 av_log(NULL, AV_LOG_ERROR, Error locking the pixel buffer.\n);
-return AVERROR_UNKNOWN;
+ret = AVERROR_UNKNOWN;
+goto fail;
 }
 
 if (CVPixelBufferIsPlanar(pixbuf)) {
@@ -89,17 +112,27 @@ static int videotoolbox_retrieve_data(AVCodecContext *s, 
AVFrame *frame)
 
 av_image_copy(vt-tmp_frame-data, vt-tmp_frame-linesize,
   (const uint8_t **)data, linesize, vt-tmp_frame-format,
-  frame-width, frame-height);
+  width, height);
 
 ret = av_frame_copy_props(vt-tmp_frame, frame);
 CVPixelBufferUnlockBaseAddress(pixbuf, kCVPixelBufferLock_ReadOnly);
-if (ret  0)
-return ret;
+if (ret  0) {
+goto fail;
+}
 
 av_frame_unref(frame);
 av_frame_move_ref(frame, vt-tmp_frame);
 
+if (videotoolbox-useAsyncDecoding) {
+av_videotoolbox_release_async_frame(async_frame);
+}
+
 return 0;
+fail:
+if (videotoolbox-useAsyncDecoding) {
+av_videotoolbox_release_async_frame(async_frame);
+}
+return ret;
 }
 
 static void videotoolbox_uninit(AVCodecContext *s)

Re: [FFmpeg-devel] [PATCH] videotoolbox: allow to enable the async decoding.

2015-08-09 Thread wm4 
On Sun, 9 Aug 2015 07:46:53 -0400
Ronald S. Bultje rsbul...@gmail.com wrote:

 Hi,
 
 On Sun, Aug 9, 2015 at 7:11 AM, Sebastien Zwickert dilar...@gmail.com
 wrote:
 
  This patch allows to use the Videotoolbox API in asynchonous mode.
 
 
 If this is for N:M (in:out), I think this should be replaced by a
 prospective avcodec async decoding API.

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

Re: [FFmpeg-devel] [PATCH] videotoolbox: allow to enable the async decoding.

2015-08-09 Thread Hendrik Leppkes 
On Sun, Aug 9, 2015 at 1:11 PM, Sebastien Zwickert dilar...@gmail.com wrote:
 This patch allows to use the Videotoolbox API in asynchonous mode.
 Note that when using async decoding the user is responsible for
 releasing the async frame.
 Moreover, an option called videotoolbox_async was added to enable
 async decoding with ffmpeg CLI.


I'm not sure about your approach. I can already see you getting loads
of problems with frame order. pts is not guaranteed to be present or
reliable, and using it for re-ordering your async frames is probably
unreliable at best.
avcodec does proper re-ordering, and the only reliable way would be to
somehow manage to associate the AVFrame avcodec is going to output
with your async frame, so that you can trust avcodecs re-ordering.

Isnt there a way to put a handle to the async frame into the AVFrame
somehow, which can then be used by the user code to get it from VT?

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