diff mbox series

[FFmpeg-devel] avdevice/decklink_dec: fix no warning message if no input signal detected

Message ID 1592492134-31436-1-git-send-email-lance.lmwang@gmail.com
State New
Headers show
Series [FFmpeg-devel] avdevice/decklink_dec: fix no warning message if no input signal detected | expand

Checks

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

Commit Message

Lance Wang June 18, 2020, 2:55 p.m. UTC 
From: Limin Wang <lance.lmwang@gmail.com>

Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
---
 libavdevice/decklink_dec.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Marton Balint June 18, 2020, 5:37 p.m. UTC | #1 
On Thu, 18 Jun 2020, lance.lmwang@gmail.com wrote:

> From: Limin Wang <lance.lmwang@gmail.com>
>
> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> ---
> libavdevice/decklink_dec.cpp | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp
> index 82106aa..90569dc 100644
> --- a/libavdevice/decklink_dec.cpp
> +++ b/libavdevice/decklink_dec.cpp
> @@ -751,7 +751,7 @@ HRESULT decklink_input_callback::VideoInputFrameArrived(
>                 }
>             }
> 
> -            if (!no_video) {
> +            if (no_video) {
>                 av_log(avctx, AV_LOG_WARNING, "Frame received (#%lu) - No input signal detected "
>                         "- Frames dropped %u\n", ctx->frameCount, ++ctx->dropped);
>             }

No, this is good as is, the !no_video condition ensures that it is only 
logged once when the input actually disappears, and not for every 
consecutive frame with no input.

Regards,
Marton
Lance Wang June 18, 2020, 11:24 p.m. UTC | #2 
On Thu, Jun 18, 2020 at 07:37:50PM +0200, Marton Balint wrote:
> 
> 
> On Thu, 18 Jun 2020, lance.lmwang@gmail.com wrote:
> 
> >From: Limin Wang <lance.lmwang@gmail.com>
> >
> >Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> >---
> >libavdevice/decklink_dec.cpp | 2 +-
> >1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp
> >index 82106aa..90569dc 100644
> >--- a/libavdevice/decklink_dec.cpp
> >+++ b/libavdevice/decklink_dec.cpp
> >@@ -751,7 +751,7 @@ HRESULT decklink_input_callback::VideoInputFrameArrived(
> >                }
> >            }
> >
> >-            if (!no_video) {
> >+            if (no_video) {
> >                av_log(avctx, AV_LOG_WARNING, "Frame received (#%lu) - No input signal detected "
> >                        "- Frames dropped %u\n", ctx->frameCount, ++ctx->dropped);
> >            }
> 
> No, this is good as is, the !no_video condition ensures that it is
> only logged once when the input actually disappears, and not for
> every consecutive frame with no input.

It's good point, but if no input signal after run, no any warning message. I
don't think it's helpful to check the signal status, also the dropped message 
isn't correct then, as it'll not count for every dropped frame by the condition.



> 
> Regards,
> Marton
> _______________________________________________
> 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".
Marton Balint June 19, 2020, 8:46 p.m. UTC | #3 
On Fri, 19 Jun 2020, Limin Wang wrote:

> On Thu, Jun 18, 2020 at 07:37:50PM +0200, Marton Balint wrote:
>> 
>> 
>> On Thu, 18 Jun 2020, lance.lmwang@gmail.com wrote:
>> 
>> >From: Limin Wang <lance.lmwang@gmail.com>
>> >
>> >Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
>> >---
>> >libavdevice/decklink_dec.cpp | 2 +-
>> >1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> >diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp
>> >index 82106aa..90569dc 100644
>> >--- a/libavdevice/decklink_dec.cpp
>> >+++ b/libavdevice/decklink_dec.cpp
>> >@@ -751,7 +751,7 @@ HRESULT decklink_input_callback::VideoInputFrameArrived(
>> >                }
>> >            }
>> >
>> >-            if (!no_video) {
>> >+            if (no_video) {
>> >                av_log(avctx, AV_LOG_WARNING, "Frame received (#%lu) - No input signal detected "
>> >                        "- Frames dropped %u\n", ctx->frameCount, ++ctx->dropped);
>> >            }
>> 
>> No, this is good as is, the !no_video condition ensures that it is
>> only logged once when the input actually disappears, and not for
>> every consecutive frame with no input.
>
> It's good point, but if no input signal after run, no any warning message.

I don't understand what you are saying here.

> I don't think it's helpful to check the signal status,

What do you mean?

> also the dropped message 
> isn't correct then, as it'll not count for every dropped frame by the condition.

That is true, a patch which fixes the number of dropped frames only is 
welcome.

Thanks,
Marton
Lance Wang June 19, 2020, 11:10 p.m. UTC | #4 
On Fri, Jun 19, 2020 at 10:46:33PM +0200, Marton Balint wrote:
> 
> 
> On Fri, 19 Jun 2020, Limin Wang wrote:
> 
> > On Thu, Jun 18, 2020 at 07:37:50PM +0200, Marton Balint wrote:
> > > 
> > > 
> > > On Thu, 18 Jun 2020, lance.lmwang@gmail.com wrote:
> > > 
> > > >From: Limin Wang <lance.lmwang@gmail.com>
> > > >
> > > >Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > > >---
> > > >libavdevice/decklink_dec.cpp | 2 +-
> > > >1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > >diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp
> > > >index 82106aa..90569dc 100644
> > > >--- a/libavdevice/decklink_dec.cpp
> > > >+++ b/libavdevice/decklink_dec.cpp
> > > >@@ -751,7 +751,7 @@ HRESULT decklink_input_callback::VideoInputFrameArrived(
> > > >                }
> > > >            }
> > > >
> > > >-            if (!no_video) {
> > > >+            if (no_video) {
> > > >                av_log(avctx, AV_LOG_WARNING, "Frame received (#%lu) - No input signal detected "
> > > >                        "- Frames dropped %u\n", ctx->frameCount, ++ctx->dropped);
> > > >            }
> > > 
> > > No, this is good as is, the !no_video condition ensures that it is
> > > only logged once when the input actually disappears, and not for
> > > every consecutive frame with no input.
> > 
> > It's good point, but if no input signal after run, no any warning message.
> 
> I don't understand what you are saying here.
> 
> > I don't think it's helpful to check the signal status,
> 
> What do you mean?

If no_video is status machine of signal, then the first status is 1(no signal).
So I think it's wrong to initialized to 0. So if the input signal is lost, 
no any warning message then. I'll submit a patch for your review.

> 
> > also the dropped message isn't correct then, as it'll not count for
> > every dropped frame by the condition.
> 
> That is true, a patch which fixes the number of dropped frames only is
> welcome.

I'll submit a patch for that. I prefer to print warning message by dropped %
25(or other mod), it's more helpful.

> 
> Thanks,
> Marton
> _______________________________________________
> 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".
Marton Balint June 19, 2020, 11:31 p.m. UTC | #5 
On Sat, 20 Jun 2020, lance.lmwang@gmail.com wrote:

> On Fri, Jun 19, 2020 at 10:46:33PM +0200, Marton Balint wrote:
>> 
>> 
>> On Fri, 19 Jun 2020, Limin Wang wrote:
>> 
>> > On Thu, Jun 18, 2020 at 07:37:50PM +0200, Marton Balint wrote:
>> > > 
>> > > 
>> > > On Thu, 18 Jun 2020, lance.lmwang@gmail.com wrote:
>> > > 
>> > > >From: Limin Wang <lance.lmwang@gmail.com>
>> > > >
>> > > >Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
>> > > >---
>> > > >libavdevice/decklink_dec.cpp | 2 +-
>> > > >1 file changed, 1 insertion(+), 1 deletion(-)
>> > > >
>> > > >diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp
>> > > >index 82106aa..90569dc 100644
>> > > >--- a/libavdevice/decklink_dec.cpp
>> > > >+++ b/libavdevice/decklink_dec.cpp
>> > > >@@ -751,7 +751,7 @@ HRESULT decklink_input_callback::VideoInputFrameArrived(
>> > > >                }
>> > > >            }
>> > > >
>> > > >-            if (!no_video) {
>> > > >+            if (no_video) {
>> > > >                av_log(avctx, AV_LOG_WARNING, "Frame received (#%lu) - No input signal detected "
>> > > >                        "- Frames dropped %u\n", ctx->frameCount, ++ctx->dropped);
>> > > >            }
>> > > 
>> > > No, this is good as is, the !no_video condition ensures that it is
>> > > only logged once when the input actually disappears, and not for
>> > > every consecutive frame with no input.
>> > 
>> > It's good point, but if no input signal after run, no any warning message.
>> 
>> I don't understand what you are saying here.
>> 
>> > I don't think it's helpful to check the signal status,
>> 
>> What do you mean?
>
> If no_video is status machine of signal, then the first status is 1(no signal).
> So I think it's wrong to initialized to 0. So if the input signal is lost, 
> no any warning message then. I'll submit a patch for your review.

initially no_video = 0, so we assume there is valid signal. When there is 
no signal, we log the warning if no_video==0, and we set no_video to 1. So 
we do give warning message when the signal is lost.

>
>> 
>> > also the dropped message isn't correct then, as it'll not count for
>> > every dropped frame by the condition.
>> 
>> That is true, a patch which fixes the number of dropped frames only is
>> welcome.
>
> I'll submit a patch for that. I prefer to print warning message by dropped %
> 25(or other mod), it's more helpful.

Sorry, i don't think so.

Regards,
Marton
Lance Wang June 19, 2020, 11:55 p.m. UTC | #6 
On Sat, Jun 20, 2020 at 01:31:43AM +0200, Marton Balint wrote:
> 
> 
> On Sat, 20 Jun 2020, lance.lmwang@gmail.com wrote:
> 
> > On Fri, Jun 19, 2020 at 10:46:33PM +0200, Marton Balint wrote:
> > > 
> > > 
> > > On Fri, 19 Jun 2020, Limin Wang wrote:
> > > 
> > > > On Thu, Jun 18, 2020 at 07:37:50PM +0200, Marton Balint wrote:
> > > > > > > > > On Thu, 18 Jun 2020, lance.lmwang@gmail.com wrote:
> > > > > > > >From: Limin Wang <lance.lmwang@gmail.com>
> > > > > >
> > > > > >Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > > > > >---
> > > > > >libavdevice/decklink_dec.cpp | 2 +-
> > > > > >1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > >
> > > > > >diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp
> > > > > >index 82106aa..90569dc 100644
> > > > > >--- a/libavdevice/decklink_dec.cpp
> > > > > >+++ b/libavdevice/decklink_dec.cpp
> > > > > >@@ -751,7 +751,7 @@ HRESULT decklink_input_callback::VideoInputFrameArrived(
> > > > > >                }
> > > > > >            }
> > > > > >
> > > > > >-            if (!no_video) {
> > > > > >+            if (no_video) {
> > > > > >                av_log(avctx, AV_LOG_WARNING, "Frame received (#%lu) - No input signal detected "
> > > > > >                        "- Frames dropped %u\n", ctx->frameCount, ++ctx->dropped);
> > > > > >            }
> > > > > > > No, this is good as is, the !no_video condition ensures that
> > > it is
> > > > > only logged once when the input actually disappears, and not for
> > > > > every consecutive frame with no input.
> > > > > It's good point, but if no input signal after run, no any
> > > warning message.
> > > 
> > > I don't understand what you are saying here.
> > > 
> > > > I don't think it's helpful to check the signal status,
> > > 
> > > What do you mean?
> > 
> > If no_video is status machine of signal, then the first status is 1(no signal).
> > So I think it's wrong to initialized to 0. So if the input signal is
> > lost, no any warning message then. I'll submit a patch for your review.
> 
> initially no_video = 0, so we assume there is valid signal. When there is no
> signal, we log the warning if no_video==0, and we set no_video to 1. So we
> do give warning message when the signal is lost.

If you don't use autodetect for the format, you'll get signal lost after runn, no
warning message for the signal lost. I'm not sure it's expected behavior.

> 
> > 
> > > 
> > > > also the dropped message isn't correct then, as it'll not count for
> > > > every dropped frame by the condition.
> > > 
> > > That is true, a patch which fixes the number of dropped frames only is
> > > welcome.
> > 
> > I'll submit a patch for that. I prefer to print warning message by dropped %
> > 25(or other mod), it's more helpful.
> 
> Sorry, i don't think so.
> 
> Regards,
> Marton
> _______________________________________________
> 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".
Marton Balint June 20, 2020, 6:39 p.m. UTC | #7 
On Sat, 20 Jun 2020, lance.lmwang@gmail.com wrote:

> On Sat, Jun 20, 2020 at 01:31:43AM +0200, Marton Balint wrote:
>> 
>> 
>> On Sat, 20 Jun 2020, lance.lmwang@gmail.com wrote:
>> 
>> > On Fri, Jun 19, 2020 at 10:46:33PM +0200, Marton Balint wrote:
>> > > 
>> > > 
>> > > On Fri, 19 Jun 2020, Limin Wang wrote:
>> > > 
>> > > > On Thu, Jun 18, 2020 at 07:37:50PM +0200, Marton Balint wrote:
>> > > > > > > > > On Thu, 18 Jun 2020, lance.lmwang@gmail.com wrote:
>> > > > > > > >From: Limin Wang <lance.lmwang@gmail.com>
>> > > > > >
>> > > > > >Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
>> > > > > >---
>> > > > > >libavdevice/decklink_dec.cpp | 2 +-
>> > > > > >1 file changed, 1 insertion(+), 1 deletion(-)
>> > > > > >
>> > > > > >diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp
>> > > > > >index 82106aa..90569dc 100644
>> > > > > >--- a/libavdevice/decklink_dec.cpp
>> > > > > >+++ b/libavdevice/decklink_dec.cpp
>> > > > > >@@ -751,7 +751,7 @@ HRESULT decklink_input_callback::VideoInputFrameArrived(
>> > > > > >                }
>> > > > > >            }
>> > > > > >
>> > > > > >-            if (!no_video) {
>> > > > > >+            if (no_video) {
>> > > > > >                av_log(avctx, AV_LOG_WARNING, "Frame received (#%lu) - No input signal detected "
>> > > > > >                        "- Frames dropped %u\n", ctx->frameCount, ++ctx->dropped);
>> > > > > >            }
>> > > > > > > No, this is good as is, the !no_video condition ensures that
>> > > it is
>> > > > > only logged once when the input actually disappears, and not for
>> > > > > every consecutive frame with no input.
>> > > > > It's good point, but if no input signal after run, no any
>> > > warning message.
>> > > 
>> > > I don't understand what you are saying here.
>> > > 
>> > > > I don't think it's helpful to check the signal status,
>> > > 
>> > > What do you mean?
>> > 
>> > If no_video is status machine of signal, then the first status is 1(no signal).
>> > So I think it's wrong to initialized to 0. So if the input signal is
>> > lost, no any warning message then. I'll submit a patch for your review.
>> 
>> initially no_video = 0, so we assume there is valid signal. When there is no
>> signal, we log the warning if no_video==0, and we set no_video to 1. So we
>> do give warning message when the signal is lost.
>
> If you don't use autodetect for the format, you'll get signal lost after runn, no
> warning message for the signal lost. I'm not sure it's expected behavior.

Did you actually test this? Because for me that is simply not the case.

Regards,
Marton
Lance Wang June 20, 2020, 10:57 p.m. UTC | #8 
On Sat, Jun 20, 2020 at 08:39:00PM +0200, Marton Balint wrote:
> 
> 
> On Sat, 20 Jun 2020, lance.lmwang@gmail.com wrote:
> 
> > On Sat, Jun 20, 2020 at 01:31:43AM +0200, Marton Balint wrote:
> > > 
> > > 
> > > On Sat, 20 Jun 2020, lance.lmwang@gmail.com wrote:
> > > 
> > > > On Fri, Jun 19, 2020 at 10:46:33PM +0200, Marton Balint wrote:
> > > > > > > > > On Fri, 19 Jun 2020, Limin Wang wrote:
> > > > > > > > On Thu, Jun 18, 2020 at 07:37:50PM +0200, Marton Balint
> > > wrote:
> > > > > > > > > > > On Thu, 18 Jun 2020, lance.lmwang@gmail.com wrote:
> > > > > > > > > >From: Limin Wang <lance.lmwang@gmail.com>
> > > > > > > >
> > > > > > > >Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > > > > > > >---
> > > > > > > >libavdevice/decklink_dec.cpp | 2 +-
> > > > > > > >1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > > > >
> > > > > > > >diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp
> > > > > > > >index 82106aa..90569dc 100644
> > > > > > > >--- a/libavdevice/decklink_dec.cpp
> > > > > > > >+++ b/libavdevice/decklink_dec.cpp
> > > > > > > >@@ -751,7 +751,7 @@ HRESULT decklink_input_callback::VideoInputFrameArrived(
> > > > > > > >                }
> > > > > > > >            }
> > > > > > > >
> > > > > > > >-            if (!no_video) {
> > > > > > > >+            if (no_video) {
> > > > > > > >                av_log(avctx, AV_LOG_WARNING, "Frame received (#%lu) - No input signal detected "
> > > > > > > >                        "- Frames dropped %u\n", ctx->frameCount, ++ctx->dropped);
> > > > > > > >            }
> > > > > > > > > No, this is good as is, the !no_video condition ensures that
> > > > > it is
> > > > > > > only logged once when the input actually disappears, and not for
> > > > > > > every consecutive frame with no input.
> > > > > > > It's good point, but if no input signal after run, no any
> > > > > warning message.
> > > > > > > I don't understand what you are saying here.
> > > > > > > > I don't think it's helpful to check the signal status,
> > > > > > > What do you mean?
> > > > > If no_video is status machine of signal, then the first status
> > > is 1(no signal).
> > > > So I think it's wrong to initialized to 0. So if the input signal is
> > > > lost, no any warning message then. I'll submit a patch for your review.
> > > 
> > > initially no_video = 0, so we assume there is valid signal. When there is no
> > > signal, we log the warning if no_video==0, and we set no_video to 1. So we
> > > do give warning message when the signal is lost.
> > 
> > If you don't use autodetect for the format, you'll get signal lost after runn, no
> > warning message for the signal lost. I'm not sure it's expected behavior.
> 
> Did you actually test this? Because for me that is simply not the case.

Sorry, retest it, it's have one warning message. At first, I'm testing the timecode
signal, so it's flooded by that.

> 
> Regards,
> Marton
> _______________________________________________
> 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/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp
index 82106aa..90569dc 100644
--- a/libavdevice/decklink_dec.cpp
+++ b/libavdevice/decklink_dec.cpp
@@ -751,7 +751,7 @@  HRESULT decklink_input_callback::VideoInputFrameArrived(
                 }
             }
 
-            if (!no_video) {
+            if (no_video) {
                 av_log(avctx, AV_LOG_WARNING, "Frame received (#%lu) - No input signal detected "
                         "- Frames dropped %u\n", ctx->frameCount, ++ctx->dropped);
             }