diff mbox series

[FFmpeg-devel] libavcodec/videotoolbox: let VideoToolbox choose a decoder for us

Message ID HK0PR03MB36519FFCB1F06A69014510B6DA4F0@HK0PR03MB3651.apcprd03.prod.outlook.com
State New
Headers show
Series [FFmpeg-devel] libavcodec/videotoolbox: let VideoToolbox choose a decoder for us | expand

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Wang Chuan Aug. 1, 2020, 3:25 p.m. UTC
If we fail to create a decoder specified by ourself, then try to
let VideoToolbox pick a proper one for us.

Signed-off-by: Wang Chuan <ouchuanm@outlook.com>
---
 libavcodec/videotoolbox.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Mark Thompson Aug. 1, 2020, 3:50 p.m. UTC | #1
On 01/08/2020 16:25, Wang Chuan wrote:
> If we fail to create a decoder specified by ourself, then try to
> let VideoToolbox pick a proper one for us.
> 
> Signed-off-by: Wang Chuan <ouchuanm@outlook.com>
> ---
>   libavcodec/videotoolbox.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/libavcodec/videotoolbox.c b/libavcodec/videotoolbox.c
> index 8773de3393..9077647e25 100644
> --- a/libavcodec/videotoolbox.c
> +++ b/libavcodec/videotoolbox.c
> @@ -837,6 +837,10 @@ static int videotoolbox_start(AVCodecContext *avctx)
>                                             &decoder_cb,               // outputCallback
>                                             &videotoolbox->session);   // decompressionSessionOut
>   
> +    // if we cannot create a decode session specified by ourself, then that videotoolbox pick one for us
> +    if (status == kVTVideoDecoderNotAvailableNowErr)
> +        status = VTDecompressionSessionCreate(NULL, videotoolbox->cm_fmt_desc, NULL, buf_attr, &decoder_cb, &videotoolbox->session);
> +
>       if (decoder_spec)
>           CFRelease(decoder_spec);
>       if (buf_attr)
> 

Can you explain why?

Looking at the code, it seems like the main consequence of this is to allow decoders to be created which have not been given the extradata header, and that isn't obviously a positive change.  (Does 
decoding a stream with a global header still work if this path is taken?)

- Mark
Wang Chuan Aug. 1, 2020, 4:09 p.m. UTC | #2
I’m just looking at the bug https://trac.ffmpeg.org/ticket/8789
And I found that in this case, VTDecompressionSessionCreate will fail and return  kVTVideoDecoderNotAvailableNowErr

While according to the document of [VTDecompressionSessionCreate], it is possible to give a null
And let videotoolbox to choose a decoder.
https://developer.apple.com/documentation/videotoolbox/1536134-vtdecompressionsessioncreate?language=occ

So I think it is worth to have a second try(if we fail to create decode session in the first time).
On Aug 1, 2020, 11:50 PM +0800, Mark Thompson <sw@jkqxz.net>, wrote:
> On 01/08/2020 16:25, Wang Chuan wrote:
> > If we fail to create a decoder specified by ourself, then try to
> > let VideoToolbox pick a proper one for us.
> >
> > Signed-off-by: Wang Chuan <ouchuanm@outlook.com>
> > ---
> > libavcodec/videotoolbox.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/libavcodec/videotoolbox.c b/libavcodec/videotoolbox.c
> > index 8773de3393..9077647e25 100644
> > --- a/libavcodec/videotoolbox.c
> > +++ b/libavcodec/videotoolbox.c
> > @@ -837,6 +837,10 @@ static int videotoolbox_start(AVCodecContext *avctx)
> > &decoder_cb, // outputCallback
> > &videotoolbox->session); // decompressionSessionOut
> >
> > + // if we cannot create a decode session specified by ourself, then that videotoolbox pick one for us
> > + if (status == kVTVideoDecoderNotAvailableNowErr)
> > + status = VTDecompressionSessionCreate(NULL, videotoolbox->cm_fmt_desc, NULL, buf_attr, &decoder_cb, &videotoolbox->session);
> > +
> > if (decoder_spec)
> > CFRelease(decoder_spec);
> > if (buf_attr)
> >
>
> Can you explain why?
>
> Looking at the code, it seems like the main consequence of this is to allow decoders to be created which have not been given the extradata header, and that isn't obviously a positive change. (Does
> decoding a stream with a global header still work if this path is taken?)
>
> - Mark
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Hendrik Leppkes Aug. 1, 2020, 5:11 p.m. UTC | #3
On Sat, Aug 1, 2020 at 6:25 PM 王 氚 <ouchuanm@outlook.com> wrote:
>
> I’m just looking at the bug https://trac.ffmpeg.org/ticket/8789
> And I found that in this case, VTDecompressionSessionCreate will fail and return  kVTVideoDecoderNotAvailableNowErr
>
> While according to the document of [VTDecompressionSessionCreate], it is possible to give a null
> And let videotoolbox to choose a decoder.
> https://developer.apple.com/documentation/videotoolbox/1536134-vtdecompressionsessioncreate?language=occ
>

Can you explain under what circumstances this might occur, and why the
second try would work?
And once we know that, can't we just improve the first attempt?

- Hendrik
Wang Chuan Aug. 2, 2020, 5:54 a.m. UTC | #4
Just dig it a little bit, and I found that the first attempt of [VTDecompressionSessionCreate] is always ok, if we try to decode some normal size of video(for example, 640x480, 1280x720...)

But it will fail, it we try to decode h264 video with some special size(for example, 300x180).

So it seems like there are some limitations in [VTDecompressionSessionCreate], but if we give VideoToolbox freedom to choose a decoder,  it's always OK.

And I try to find some information about this, but there isn’t so many detail information about this in apple’s document:(

Any suggestions?
On Aug 2, 2020, 1:11 AM +0800, Hendrik Leppkes <h.leppkes@gmail.com>, wrote:
> On Sat, Aug 1, 2020 at 6:25 PM 王 氚 <ouchuanm@outlook.com> wrote:
> >
> > I’m just looking at the bug https://trac.ffmpeg.org/ticket/8789
> > And I found that in this case, VTDecompressionSessionCreate will fail and return kVTVideoDecoderNotAvailableNowErr
> >
> > While according to the document of [VTDecompressionSessionCreate], it is possible to give a null
> > And let videotoolbox to choose a decoder.
> > https://developer.apple.com/documentation/videotoolbox/1536134-vtdecompressionsessioncreate?language=occ
> >
>
> Can you explain under what circumstances this might occur, and why the
> second try would work?
> And once we know that, can't we just improve the first attempt?
>
> - Hendrik
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Hendrik Leppkes Aug. 2, 2020, 7:03 a.m. UTC | #5
On Sun, Aug 2, 2020 at 7:54 AM 王 氚 <ouchuanm@outlook.com> wrote:
>
> Just dig it a little bit, and I found that the first attempt of [VTDecompressionSessionCreate] is always ok, if we try to decode some normal size of video(for example, 640x480, 1280x720...)
>
> But it will fail, it we try to decode h264 video with some special size(for example, 300x180).
>
> So it seems like there are some limitations in [VTDecompressionSessionCreate], but if we give VideoToolbox freedom to choose a decoder,  it's always OK.
>

What other decoders could it possibly pick? Is it still using hardware
decoding, or is that a software decoder then?
Our software decoders are likely better then running software decoding
through VT, so falling back to that would be better.

- Hendrik
Wang Chuan Aug. 2, 2020, 8:45 a.m. UTC | #6
I can observe GPU consumption and lower CPU consumption in this case just like decoding 640x480 which obviously using hardware decoding(since it succeed in first attempt) in my MacBook.

So from my point of view, it still uses hardware decoding.

I’m still not sure whether or not all apple’s device following this behavior. But maybe this is user’s responsibility to choose which type of decoding they want. If they want to use hardware decoding, we can try to find one, if they feel not good, it’s freely to change it, especially for the case https://trac.ffmpeg.org/ticket/8789
On Aug 2, 2020, 3:09 PM +0800, Hendrik Leppkes <h.leppkes@gmail.com>, wrote:
> On Sun, Aug 2, 2020 at 7:54 AM 王 氚 <ouchuanm@outlook.com> wrote:
> >
> > Just dig it a little bit, and I found that the first attempt of [VTDecompressionSessionCreate] is always ok, if we try to decode some normal size of video(for example, 640x480, 1280x720...)
> >
> > But it will fail, it we try to decode h264 video with some special size(for example, 300x180).
> >
> > So it seems like there are some limitations in [VTDecompressionSessionCreate], but if we give VideoToolbox freedom to choose a decoder, it's always OK.
> >
>
> What other decoders could it possibly pick? Is it still using hardware
> decoding, or is that a software decoder then?
> Our software decoders are likely better then running software decoding
> through VT, so falling back to that would be better.
>
> - Hendrik
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff mbox series

Patch

diff --git a/libavcodec/videotoolbox.c b/libavcodec/videotoolbox.c
index 8773de3393..9077647e25 100644
--- a/libavcodec/videotoolbox.c
+++ b/libavcodec/videotoolbox.c
@@ -837,6 +837,10 @@  static int videotoolbox_start(AVCodecContext *avctx)
                                           &decoder_cb,               // outputCallback
                                           &videotoolbox->session);   // decompressionSessionOut
 
+    // if we cannot create a decode session specified by ourself, then that videotoolbox pick one for us
+    if (status == kVTVideoDecoderNotAvailableNowErr)
+        status = VTDecompressionSessionCreate(NULL, videotoolbox->cm_fmt_desc, NULL, buf_attr, &decoder_cb, &videotoolbox->session);
+
     if (decoder_spec)
         CFRelease(decoder_spec);
     if (buf_attr)