[FFmpeg-devel] [PATCH WIP 0/9] Refactor DNN

[FFmpeg-devel] [PATCH WIP 0/9] Refactor DNN

[Zhao Zhili]

From: Zhao Zhili <zhilizhao@tencent.com>

During the refactor progress, I have found some serious issues, which
is not resolved by the patchset:

1. Tensorflow backend is broken.

I think it doesn't work since 2021 at least. For example, it destroy a
thread and create a new thread for each frame, and it destroy an invalid
thread at the first frame:


    pthread_join(async_module->thread_id, &status);
    if (status == DNN_ASYNC_FAIL) {
        av_log(ctx, AV_LOG_ERROR, "Unable to start inference as previous inference failed.\n");
        return DNN_GENERIC_ERROR;
    }
    ret = pthread_create(&async_module->thread_id, NULL, async_thread_routine, async_module);


2. Openvino V1 doesn't compile. It doesn't compile and no one complains,
I think it's a hint to just keep the code for V2.

3. Error handling. It's easy to crash with incorrect command line arguments.

I don't have enough test case. Please share your test case and help on test.

Zhao Zhili (9):
  avfilter/dnn: Refactor DNN parameter configuration system
  avfilter/dnn_backend_openvino: Fix free context at random place
  avfilter/dnn_backend_openvino: simplify memory allocation
  avfilter/dnn_backend_tf: Remove one level of indentation
  avfilter/dnn_backend_tf: Fix free context at random place
  avfilter/dnn_backend_tf: Simplify memory allocation
  avfilter/dnn_backend_torch: Simplify memory allocation
  avfilter/dnn: Remove a level of dereference
  avfilter/dnn: Use dnn_backend_info_list to search for dnn module

 libavfilter/dnn/dnn_backend_common.h   |  13 +-
 libavfilter/dnn/dnn_backend_openvino.c | 210 ++++++++++---------------
 libavfilter/dnn/dnn_backend_tf.c       | 194 ++++++++++-------------
 libavfilter/dnn/dnn_backend_torch.cpp  | 112 +++++--------
 libavfilter/dnn/dnn_interface.c        | 107 ++++++++++---
 libavfilter/dnn_filter_common.c        |  38 ++++-
 libavfilter/dnn_filter_common.h        |  37 ++---
 libavfilter/dnn_interface.h            |  73 +++++++--
 libavfilter/vf_derain.c                |   5 +-
 libavfilter/vf_dnn_classify.c          |   3 +-
 libavfilter/vf_dnn_detect.c            |   3 +-
 libavfilter/vf_dnn_processing.c        |   3 +-
 libavfilter/vf_sr.c                    |   5 +-
 13 files changed, 428 insertions(+), 375 deletions(-)

-- 
2.34.1

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH WIP 0/9] Refactor DNN

[Guo, Yejun]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Zhao
> Zhili
> Sent: Sunday, April 28, 2024 12:42 AM
> To: ffmpeg-devel@ffmpeg.org
> Cc: Zhao Zhili <zhilizhao@tencent.com>
> Subject: [FFmpeg-devel] [PATCH WIP 0/9] Refactor DNN
> 
> From: Zhao Zhili <zhilizhao@tencent.com>
> 
> During the refactor progress, I have found some serious issues, which is not
> resolved by the patchset:
> 
> 1. Tensorflow backend is broken.
> 
> I think it doesn't work since 2021 at least. For example, it destroy a thread and
> create a new thread for each frame, and it destroy an invalid thread at the first
> frame:

It works from the day that code is merged, till today. It is by design to keep the
code simplicity by using the feature that pthread_join accepts a parameter that
is not a joinable thread.

Please share more info if you experienced a real case that it does not work.
> 
> 
>     pthread_join(async_module->thread_id, &status);
>     if (status == DNN_ASYNC_FAIL) {
>         av_log(ctx, AV_LOG_ERROR, "Unable to start inference as previous
> inference failed.\n");
>         return DNN_GENERIC_ERROR;
>     }
>     ret = pthread_create(&async_module->thread_id, NULL,
> async_thread_routine, async_module);
> 
> 
> 2. Openvino V1 doesn't compile. It doesn't compile and no one complains, I
> think it's a hint to just keep the code for V2.

In plan, and patch is welcome.

> 
> 3. Error handling. It's easy to crash with incorrect command line arguments.

Thanks, will review your patchset one by one, it may take some time.

> 
> I don't have enough test case. Please share your test case and help on test.
> 
> Zhao Zhili (9):
>   avfilter/dnn: Refactor DNN parameter configuration system
>   avfilter/dnn_backend_openvino: Fix free context at random place
>   avfilter/dnn_backend_openvino: simplify memory allocation
>   avfilter/dnn_backend_tf: Remove one level of indentation
>   avfilter/dnn_backend_tf: Fix free context at random place
>   avfilter/dnn_backend_tf: Simplify memory allocation
>   avfilter/dnn_backend_torch: Simplify memory allocation
>   avfilter/dnn: Remove a level of dereference
>   avfilter/dnn: Use dnn_backend_info_list to search for dnn module
> 
>  libavfilter/dnn/dnn_backend_common.h   |  13 +-
>  libavfilter/dnn/dnn_backend_openvino.c | 210 ++++++++++---------------
>  libavfilter/dnn/dnn_backend_tf.c       | 194 ++++++++++-------------
>  libavfilter/dnn/dnn_backend_torch.cpp  | 112 +++++--------
>  libavfilter/dnn/dnn_interface.c        | 107 ++++++++++---
>  libavfilter/dnn_filter_common.c        |  38 ++++-
>  libavfilter/dnn_filter_common.h        |  37 ++---
>  libavfilter/dnn_interface.h            |  73 +++++++--
>  libavfilter/vf_derain.c                |   5 +-
>  libavfilter/vf_dnn_classify.c          |   3 +-
>  libavfilter/vf_dnn_detect.c            |   3 +-
>  libavfilter/vf_dnn_processing.c        |   3 +-
>  libavfilter/vf_sr.c                    |   5 +-
>  13 files changed, 428 insertions(+), 375 deletions(-)
> 
> --
> 2.34.1
> 
> _______________________________________________
> 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".
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH WIP 0/9] Refactor DNN

[Zhao Zhili]

> On Apr 28, 2024, at 18:34, Guo, Yejun <yejun.guo-at-intel.com@ffmpeg.org> wrote:
> 
>> -----Original Message-----
>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org <mailto:ffmpeg-devel-bounces@ffmpeg.org>> On Behalf Of Zhao
>> Zhili
>> Sent: Sunday, April 28, 2024 12:42 AM
>> To: ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org>
>> Cc: Zhao Zhili <zhilizhao@tencent.com <mailto:zhilizhao@tencent.com>>
>> Subject: [FFmpeg-devel] [PATCH WIP 0/9] Refactor DNN
>> 
>> From: Zhao Zhili <zhilizhao@tencent.com>
>> 
>> During the refactor progress, I have found some serious issues, which is not
>> resolved by the patchset:
>> 
>> 1. Tensorflow backend is broken.
>> 
>> I think it doesn't work since 2021 at least. For example, it destroy a thread and
>> create a new thread for each frame, and it destroy an invalid thread at the first
>> frame:
> 
> It works from the day that code is merged, till today. It is by design to keep the
> code simplicity by using the feature that pthread_join accepts a parameter that
> is not a joinable thread.
> 
> Please share more info if you experienced a real case that it does not work.

It will abort if ASSERT_LEVEL > 1.

#define ASSERT_PTHREAD_ABORT(func, ret) do {                            \
    char errbuf[AV_ERROR_MAX_STRING_SIZE] = "";                         \
    av_log(NULL, AV_LOG_FATAL, AV_STRINGIFY(func)                       \
           " failed with error: %s\n",                                  \
           av_make_error_string(errbuf, AV_ERROR_MAX_STRING_SIZE,       \
                                AVERROR(ret)));                         \
    abort();                                                            \
} while (0)

I think the check is there just to prevent call pthread_join(0, &ptr) by accident, so we
shouldn’t do that on purpose.

>> 
>> 
>>    pthread_join(async_module->thread_id, &status);
>>    if (status == DNN_ASYNC_FAIL) {
>>        av_log(ctx, AV_LOG_ERROR, "Unable to start inference as previous
>> inference failed.\n");
>>        return DNN_GENERIC_ERROR;
>>    }
>>    ret = pthread_create(&async_module->thread_id, NULL,
>> async_thread_routine, async_module);
>> 
>> 
>> 2. Openvino V1 doesn't compile. It doesn't compile and no one complains, I
>> think it's a hint to just keep the code for V2.
> 
> In plan, and patch is welcome.
> 
>> 
>> 3. Error handling. It's easy to crash with incorrect command line arguments.
> 
> Thanks, will review your patchset one by one, it may take some time.
> 
>> 
>> I don't have enough test case. Please share your test case and help on test.
>> 
>> Zhao Zhili (9):
>>  avfilter/dnn: Refactor DNN parameter configuration system
>>  avfilter/dnn_backend_openvino: Fix free context at random place
>>  avfilter/dnn_backend_openvino: simplify memory allocation
>>  avfilter/dnn_backend_tf: Remove one level of indentation
>>  avfilter/dnn_backend_tf: Fix free context at random place
>>  avfilter/dnn_backend_tf: Simplify memory allocation
>>  avfilter/dnn_backend_torch: Simplify memory allocation
>>  avfilter/dnn: Remove a level of dereference
>>  avfilter/dnn: Use dnn_backend_info_list to search for dnn module
>> 
>> libavfilter/dnn/dnn_backend_common.h   |  13 +-
>> libavfilter/dnn/dnn_backend_openvino.c | 210 ++++++++++---------------
>> libavfilter/dnn/dnn_backend_tf.c       | 194 ++++++++++-------------
>> libavfilter/dnn/dnn_backend_torch.cpp  | 112 +++++--------
>> libavfilter/dnn/dnn_interface.c        | 107 ++++++++++---
>> libavfilter/dnn_filter_common.c        |  38 ++++-
>> libavfilter/dnn_filter_common.h        |  37 ++---
>> libavfilter/dnn_interface.h            |  73 +++++++--
>> libavfilter/vf_derain.c                |   5 +-
>> libavfilter/vf_dnn_classify.c          |   3 +-
>> libavfilter/vf_dnn_detect.c            |   3 +-
>> libavfilter/vf_dnn_processing.c        |   3 +-
>> libavfilter/vf_sr.c                    |   5 +-
>> 13 files changed, 428 insertions(+), 375 deletions(-)
>> 
>> --
>> 2.34.1
>> 
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org>
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> 
>> To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org <mailto:ffmpeg-devel-request@ffmpeg.org>
>> with subject "unsubscribe".
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org>
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org <mailto:ffmpeg-devel-request@ffmpeg.org> with subject "unsubscribe".

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH WIP 0/9] Refactor DNN

[Paul B Mahol]

Extremely low quality filters, both in source code quality and
performance/security and output quality should be queued for removal.
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH WIP 0/9] Refactor DNN

[Zhao Zhili]

> On Apr 28, 2024, at 18:58, Paul B Mahol <onemda@gmail.com> wrote:
> 
> Extremely low quality filters, both in source code quality and
> performance/security and output quality should be queued for removal.

I don’t think there is any replacement for the function provided by dnn_detect,
which is very important for ROI encoding.

> _______________________________________________
> 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".

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH WIP 0/9] Refactor DNN

[Chen, Wenbin]

> > On Apr 28, 2024, at 18:58, Paul B Mahol <onemda@gmail.com> wrote:
> >
> > Extremely low quality filters, both in source code quality and
> > performance/security and output quality should be queued for removal.

These filters cannot be removed because there are users using them.
What are your suggestions to improve them if you think they are low quality filters?

- Wenbin

> 
> I don’t think there is any replacement for the function provided by
> dnn_detect,
> which is very important for ROI encoding.
> 
> > _______________________________________________
> > 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".
> 
> _______________________________________________
> 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".
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH WIP 0/9] Refactor DNN

[Guo, Yejun]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Zhao
> Zhili
> Sent: Sunday, April 28, 2024 6:55 PM
> To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH WIP 0/9] Refactor DNN
> 
> 
> 
> > On Apr 28, 2024, at 18:34, Guo, Yejun <yejun.guo-at-
> intel.com@ffmpeg.org> wrote:
> >
> >> -----Original Message-----
> >> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org
> >> <mailto:ffmpeg-devel-bounces@ffmpeg.org>> On Behalf Of Zhao Zhili
> >> Sent: Sunday, April 28, 2024 12:42 AM
> >> To: ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org>
> >> Cc: Zhao Zhili <zhilizhao@tencent.com <mailto:zhilizhao@tencent.com>>
> >> Subject: [FFmpeg-devel] [PATCH WIP 0/9] Refactor DNN
> >>
> >> From: Zhao Zhili <zhilizhao@tencent.com>
> >>
> >> During the refactor progress, I have found some serious issues, which
> >> is not resolved by the patchset:
> >>
> >> 1. Tensorflow backend is broken.
> >>
> >> I think it doesn't work since 2021 at least. For example, it destroy
> >> a thread and create a new thread for each frame, and it destroy an
> >> invalid thread at the first
> >> frame:
> >
> > It works from the day that code is merged, till today. It is by design
> > to keep the code simplicity by using the feature that pthread_join
> > accepts a parameter that is not a joinable thread.
> >
> > Please share more info if you experienced a real case that it does not work.
> 
> It will abort if ASSERT_LEVEL > 1.
> 
> #define ASSERT_PTHREAD_ABORT(func, ret) do {                            \
>     char errbuf[AV_ERROR_MAX_STRING_SIZE] = "";                         \
>     av_log(NULL, AV_LOG_FATAL, AV_STRINGIFY(func)                       \
>            " failed with error: %s\n",                                  \
>            av_make_error_string(errbuf, AV_ERROR_MAX_STRING_SIZE,       \
>                                 AVERROR(ret)));                         \
>     abort();                                                            \
> } while (0)
> 
> I think the check is there just to prevent call pthread_join(0, &ptr) by accident,
> so we shouldn’t do that on purpose.
> 
Nice catch with configure assert level > 1, will fix, and patch also welcome, thanks.
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH WIP 0/9] Refactor DNN

[Zhao Zhili]

> On Apr 29, 2024, at 18:29, Guo, Yejun <yejun.guo-at-intel.com@ffmpeg.org> wrote:
> 
> 
> 
>> -----Original Message-----
>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Zhao
>> Zhili
>> Sent: Sunday, April 28, 2024 6:55 PM
>> To: FFmpeg development discussions and patches <ffmpeg-
>> devel@ffmpeg.org>
>> Subject: Re: [FFmpeg-devel] [PATCH WIP 0/9] Refactor DNN
>> 
>> 
>> 
>>> On Apr 28, 2024, at 18:34, Guo, Yejun <yejun.guo-at-
>> intel.com@ffmpeg.org> wrote:
>>> 
>>>> -----Original Message-----
>>>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org
>>>> <mailto:ffmpeg-devel-bounces@ffmpeg.org>> On Behalf Of Zhao Zhili
>>>> Sent: Sunday, April 28, 2024 12:42 AM
>>>> To: ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org>
>>>> Cc: Zhao Zhili <zhilizhao@tencent.com <mailto:zhilizhao@tencent.com>>
>>>> Subject: [FFmpeg-devel] [PATCH WIP 0/9] Refactor DNN
>>>> 
>>>> From: Zhao Zhili <zhilizhao@tencent.com>
>>>> 
>>>> During the refactor progress, I have found some serious issues, which
>>>> is not resolved by the patchset:
>>>> 
>>>> 1. Tensorflow backend is broken.
>>>> 
>>>> I think it doesn't work since 2021 at least. For example, it destroy
>>>> a thread and create a new thread for each frame, and it destroy an
>>>> invalid thread at the first
>>>> frame:
>>> 
>>> It works from the day that code is merged, till today. It is by design
>>> to keep the code simplicity by using the feature that pthread_join
>>> accepts a parameter that is not a joinable thread.
>>> 
>>> Please share more info if you experienced a real case that it does not work.
>> 
>> It will abort if ASSERT_LEVEL > 1.
>> 
>> #define ASSERT_PTHREAD_ABORT(func, ret) do {                            \
>>    char errbuf[AV_ERROR_MAX_STRING_SIZE] = "";                         \
>>    av_log(NULL, AV_LOG_FATAL, AV_STRINGIFY(func)                       \
>>           " failed with error: %s\n",                                  \
>>           av_make_error_string(errbuf, AV_ERROR_MAX_STRING_SIZE,       \
>>                                AVERROR(ret)));                         \
>>    abort();                                                            \
>> } while (0)
>> 
>> I think the check is there just to prevent call pthread_join(0, &ptr) by accident,
>> so we shouldn’t do that on purpose.
>> 
> Nice catch with configure assert level > 1, will fix, and patch also welcome, thanks.

If I read the code correctly, it destroy a thread and create a new thread for each
frame. I think this “async” mode isn’t common in ffmpeg’s design. Create new thread
for each frame can be heavy on some platforms. We use slice threading to improve
parallel, and thread with command queue to improve throughput. In this case with
tensorflow do the heavy lift, if it doesn’t support async operation, simple synchronous
operation with tensorflow backend should be find. The “async” mode is unnecessary
and use more resource  over the benefit it provides.

> _______________________________________________
> 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".

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH WIP 0/9] Refactor DNN

[Chen, Wenbin]

> > On Apr 29, 2024, at 18:29, Guo, Yejun <yejun.guo-at-intel.com@ffmpeg.org>
> wrote:
> >
> >
> >
> >> -----Original Message-----
> >> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Zhao
> >> Zhili
> >> Sent: Sunday, April 28, 2024 6:55 PM
> >> To: FFmpeg development discussions and patches <ffmpeg-
> >> devel@ffmpeg.org>
> >> Subject: Re: [FFmpeg-devel] [PATCH WIP 0/9] Refactor DNN
> >>
> >>
> >>
> >>> On Apr 28, 2024, at 18:34, Guo, Yejun <yejun.guo-at-
> >> intel.com@ffmpeg.org> wrote:
> >>>
> >>>> -----Original Message-----
> >>>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org
> >>>> <mailto:ffmpeg-devel-bounces@ffmpeg.org>> On Behalf Of Zhao Zhili
> >>>> Sent: Sunday, April 28, 2024 12:42 AM
> >>>> To: ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org>
> >>>> Cc: Zhao Zhili <zhilizhao@tencent.com
> <mailto:zhilizhao@tencent.com>>
> >>>> Subject: [FFmpeg-devel] [PATCH WIP 0/9] Refactor DNN
> >>>>
> >>>> From: Zhao Zhili <zhilizhao@tencent.com>
> >>>>
> >>>> During the refactor progress, I have found some serious issues, which
> >>>> is not resolved by the patchset:
> >>>>
> >>>> 1. Tensorflow backend is broken.
> >>>>
> >>>> I think it doesn't work since 2021 at least. For example, it destroy
> >>>> a thread and create a new thread for each frame, and it destroy an
> >>>> invalid thread at the first
> >>>> frame:
> >>>
> >>> It works from the day that code is merged, till today. It is by design
> >>> to keep the code simplicity by using the feature that pthread_join
> >>> accepts a parameter that is not a joinable thread.
> >>>
> >>> Please share more info if you experienced a real case that it does not
> work.
> >>
> >> It will abort if ASSERT_LEVEL > 1.
> >>
> >> #define ASSERT_PTHREAD_ABORT(func, ret) do {                            \
> >>    char errbuf[AV_ERROR_MAX_STRING_SIZE] = "";                         \
> >>    av_log(NULL, AV_LOG_FATAL, AV_STRINGIFY(func)                       \
> >>           " failed with error: %s\n",                                  \
> >>           av_make_error_string(errbuf, AV_ERROR_MAX_STRING_SIZE,       \
> >>                                AVERROR(ret)));                         \
> >>    abort();                                                            \
> >> } while (0)
> >>
> >> I think the check is there just to prevent call pthread_join(0, &ptr) by
> accident,
> >> so we shouldn’t do that on purpose.
> >>
> > Nice catch with configure assert level > 1, will fix, and patch also welcome,
> thanks.
> 
> If I read the code correctly, it destroy a thread and create a new thread for
> each
> frame. I think this “async” mode isn’t common in ffmpeg’s design. Create new
> thread
> for each frame can be heavy on some platforms. We use slice threading to
> improve
> parallel, and thread with command queue to improve throughput. In this
> case with
> tensorflow do the heavy lift, if it doesn’t support async operation, simple
> synchronous
> operation with tensorflow backend should be find. The “async” mode is
> unnecessary
> and use more resource  over the benefit it provides.

I think we need to keep async support.
1. Some model cannot make full use of resource. This may be caused by tensorflow
implementation or by model design. Async has benefit on this situation.
2. Async helps to build pipeline. You don't need to wait the output. If a "synchronous" filter
followed by another "synchronous" filter, it can be the bottle neck of the whole pipeline.

The benefit on these two situations will be more obvious if model is running on GPU.( Tensorflow
has not added device support yet.)

> 
> > _______________________________________________
> > 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".
> 
> _______________________________________________
> 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".
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH WIP 0/9] Refactor DNN

[Guo, Yejun]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Chen,
> Wenbin
> Sent: Tuesday, April 30, 2024 10:55 AM
> To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH WIP 0/9] Refactor DNN
> 
> > > On Apr 29, 2024, at 18:29, Guo, Yejun
> > > <yejun.guo-at-intel.com@ffmpeg.org>
> > wrote:
> > >
> > >
> > >
> > >> -----Original Message-----
> > >> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > Zhao
> > >> Zhili
> > >> Sent: Sunday, April 28, 2024 6:55 PM
> > >> To: FFmpeg development discussions and patches <ffmpeg-
> > >> devel@ffmpeg.org>
> > >> Subject: Re: [FFmpeg-devel] [PATCH WIP 0/9] Refactor DNN
> > >>
> > >>
> > >>
> > >>> On Apr 28, 2024, at 18:34, Guo, Yejun <yejun.guo-at-
> > >> intel.com@ffmpeg.org> wrote:
> > >>>
> > >>>> -----Original Message-----
> > >>>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org
> > >>>> <mailto:ffmpeg-devel-bounces@ffmpeg.org>> On Behalf Of Zhao Zhili
> > >>>> Sent: Sunday, April 28, 2024 12:42 AM
> > >>>> To: ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org>
> > >>>> Cc: Zhao Zhili <zhilizhao@tencent.com
> > <mailto:zhilizhao@tencent.com>>
> > >>>> Subject: [FFmpeg-devel] [PATCH WIP 0/9] Refactor DNN
> > >>>>
> > >>>> From: Zhao Zhili <zhilizhao@tencent.com>
> > >>>>
> > >>>> During the refactor progress, I have found some serious issues,
> > >>>> which is not resolved by the patchset:
> > >>>>
> > >>>> 1. Tensorflow backend is broken.
> > >>>>
> > >>>> I think it doesn't work since 2021 at least. For example, it
> > >>>> destroy a thread and create a new thread for each frame, and it
> > >>>> destroy an invalid thread at the first
> > >>>> frame:
> > >>>
> > >>> It works from the day that code is merged, till today. It is by
> > >>> design to keep the code simplicity by using the feature that
> > >>> pthread_join accepts a parameter that is not a joinable thread.
> > >>>
> > >>> Please share more info if you experienced a real case that it does
> > >>> not
> > work.
> > >>
> > >> It will abort if ASSERT_LEVEL > 1.
> > >>
> > >> #define ASSERT_PTHREAD_ABORT(func, ret) do {                            \
> > >>    char errbuf[AV_ERROR_MAX_STRING_SIZE] = "";                         \
> > >>    av_log(NULL, AV_LOG_FATAL, AV_STRINGIFY(func)                       \
> > >>           " failed with error: %s\n",                                  \
> > >>           av_make_error_string(errbuf, AV_ERROR_MAX_STRING_SIZE,       \
> > >>                                AVERROR(ret)));                         \
> > >>    abort();                                                            \
> > >> } while (0)
> > >>
> > >> I think the check is there just to prevent call pthread_join(0,
> > >> &ptr) by
> > accident,
> > >> so we shouldn’t do that on purpose.
> > >>
> > > Nice catch with configure assert level > 1, will fix, and patch also
> > > welcome,
> > thanks.
> >
> > If I read the code correctly, it destroy a thread and create a new
> > thread for each frame. I think this “async” mode isn’t common in
> > ffmpeg’s design. Create new thread for each frame can be heavy on some
> > platforms. We use slice threading to improve parallel, and thread with
> > command queue to improve throughput. In this case with tensorflow do
> > the heavy lift, if it doesn’t support async operation, simple
> > synchronous operation with tensorflow backend should be find. The
> > “async” mode is unnecessary and use more resource  over the benefit it
> > provides.
> 
> I think we need to keep async support.
> 1. Some model cannot make full use of resource. This may be caused by
> tensorflow implementation or by model design. Async has benefit on this
> situation.
> 2. Async helps to build pipeline. You don't need to wait the output. If a
> "synchronous" filter followed by another "synchronous" filter, it can be the
> bottle neck of the whole pipeline.
> 
> The benefit on these two situations will be more obvious if model is running
> on GPU.( Tensorflow has not added device support yet.)

Yes, the async mode (even with current vanilla implementation) helps performance 
with the overlap of CPU and GPU. By offloading the dnn filter to GPU, the CPU can 
do other things at the same time.

For tensorflow backend, to running on GPU, just download and use the GPU version
of tensorflow c lib, no need to set with any option.
_______________________________________________
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".