Re: [FFmpeg-devel] [PATCH] avcodec/pngdec: mark previous_picture as done on end of decode_frame_common()

[Michael Niedermayer]

On Wed, Sep 30, 2015 at 08:14:10PM -0400, Ronald S. Bultje wrote:
> Hi,
> 
> On Sun, Sep 27, 2015 at 9:13 PM, Michael Niedermayer 
> wrote:
> 
> > From: Michael Niedermayer 
> >
> > Fixes deadlock with threads
> >
> > Found-by: Paul B Mahol
> > Signed-off-by: Michael Niedermayer 
> > ---
> >  libavcodec/pngdec.c |2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/libavcodec/pngdec.c b/libavcodec/pngdec.c
> > index ee11f12..743144b 100644
> > --- a/libavcodec/pngdec.c
> > +++ b/libavcodec/pngdec.c
> > @@ -1217,6 +1217,7 @@ exit_loop:
> >  }
> >  }
> >  ff_thread_report_progress(>picture, INT_MAX, 0);
> > +ff_thread_report_progress(>previous_picture, INT_MAX, 0);
> >
> >  av_frame_set_metadata(p, metadata);
> >  metadata   = NULL;
> > @@ -1225,6 +1226,7 @@ exit_loop:
> >  fail:
> >  av_dict_free();
> >  ff_thread_report_progress(>picture, INT_MAX, 0);
> > +ff_thread_report_progress(>previous_picture, INT_MAX, 0);
> >  return ret;
> >  }
> >
> > --
> > 1.7.9.5
> 
> 
> So I directly admit I didn't look very deep, so I may be missing something
> terribly obvious, but uhm. Shouldn't we mark previous_picture as done when
> it's current? Why mark it as done in the frame after?

previous_picture is directly allocated and copied into

if you can to redesign this, i wouldnt mind

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

In fact, the RIAA has been known to suggest that students drop out
of college or go to community college in order to be able to afford
settlements. -- The RIAA


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

Re: [FFmpeg-devel] [PATCH] avcodec/pngdec: mark previous_picture as done on end of decode_frame_common()

[Michael Niedermayer]

On Wed, Sep 30, 2015 at 04:38:40PM +0200, Paul B Mahol wrote:
> On 9/28/15, Michael Niedermayer  wrote:
> > From: Michael Niedermayer 
> >
> > Fixes deadlock with threads
> >
> > Found-by: Paul B Mahol
> > Signed-off-by: Michael Niedermayer 
> > ---
> >  libavcodec/pngdec.c |2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/libavcodec/pngdec.c b/libavcodec/pngdec.c
> > index ee11f12..743144b 100644
> > --- a/libavcodec/pngdec.c
> > +++ b/libavcodec/pngdec.c
> > @@ -1217,6 +1217,7 @@ exit_loop:
> >  }
> >  }
> >  ff_thread_report_progress(>picture, INT_MAX, 0);
> > +ff_thread_report_progress(>previous_picture, INT_MAX, 0);
> >
> >  av_frame_set_metadata(p, metadata);
> >  metadata   = NULL;
> > @@ -1225,6 +1226,7 @@ exit_loop:
> >  fail:
> >  av_dict_free();
> >  ff_thread_report_progress(>picture, INT_MAX, 0);
> > +ff_thread_report_progress(>previous_picture, INT_MAX, 0);
> >  return ret;
> >  }
> >
> > --
> > 1.7.9.5
> 
> lgtm

applied

thanks

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Many that live deserve death. And some that die deserve life. Can you give
it to them? Then do not be too eager to deal out death in judgement. For
even the very wise cannot see all ends. -- Gandalf


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

Re: [FFmpeg-devel] [PATCH] avcodec/pngdec: mark previous_picture as done on end of decode_frame_common()

[Ronald S. Bultje]

Hi,

On Sun, Sep 27, 2015 at 9:13 PM, Michael Niedermayer 
wrote:

> From: Michael Niedermayer 
>
> Fixes deadlock with threads
>
> Found-by: Paul B Mahol
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/pngdec.c |2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/libavcodec/pngdec.c b/libavcodec/pngdec.c
> index ee11f12..743144b 100644
> --- a/libavcodec/pngdec.c
> +++ b/libavcodec/pngdec.c
> @@ -1217,6 +1217,7 @@ exit_loop:
>  }
>  }
>  ff_thread_report_progress(>picture, INT_MAX, 0);
> +ff_thread_report_progress(>previous_picture, INT_MAX, 0);
>
>  av_frame_set_metadata(p, metadata);
>  metadata   = NULL;
> @@ -1225,6 +1226,7 @@ exit_loop:
>  fail:
>  av_dict_free();
>  ff_thread_report_progress(>picture, INT_MAX, 0);
> +ff_thread_report_progress(>previous_picture, INT_MAX, 0);
>  return ret;
>  }
>
> --
> 1.7.9.5


So I directly admit I didn't look very deep, so I may be missing something
terribly obvious, but uhm. Shouldn't we mark previous_picture as done when
it's current? Why mark it as done in the frame after?

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

Re: [FFmpeg-devel] [PATCH] avcodec/pngdec: mark previous_picture as done on end of decode_frame_common()

[Paul B Mahol]

On 9/28/15, Michael Niedermayer  wrote:
> From: Michael Niedermayer 
>
> Fixes deadlock with threads
>
> Found-by: Paul B Mahol
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/pngdec.c |2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/libavcodec/pngdec.c b/libavcodec/pngdec.c
> index ee11f12..743144b 100644
> --- a/libavcodec/pngdec.c
> +++ b/libavcodec/pngdec.c
> @@ -1217,6 +1217,7 @@ exit_loop:
>  }
>  }
>  ff_thread_report_progress(>picture, INT_MAX, 0);
> +ff_thread_report_progress(>previous_picture, INT_MAX, 0);
>
>  av_frame_set_metadata(p, metadata);
>  metadata   = NULL;
> @@ -1225,6 +1226,7 @@ exit_loop:
>  fail:
>  av_dict_free();
>  ff_thread_report_progress(>picture, INT_MAX, 0);
> +ff_thread_report_progress(>previous_picture, INT_MAX, 0);
>  return ret;
>  }
>
> --
> 1.7.9.5

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

[FFmpeg-devel] [PATCH] avcodec/pngdec: mark previous_picture as done on end of decode_frame_common()

[Michael Niedermayer]

From: Michael Niedermayer 

Fixes deadlock with threads

Found-by: Paul B Mahol
Signed-off-by: Michael Niedermayer 
---
 libavcodec/pngdec.c |2 ++
 1 file changed, 2 insertions(+)

diff --git a/libavcodec/pngdec.c b/libavcodec/pngdec.c
index ee11f12..743144b 100644
--- a/libavcodec/pngdec.c
+++ b/libavcodec/pngdec.c
@@ -1217,6 +1217,7 @@ exit_loop:
 }
 }
 ff_thread_report_progress(>picture, INT_MAX, 0);
+ff_thread_report_progress(>previous_picture, INT_MAX, 0);
 
 av_frame_set_metadata(p, metadata);
 metadata   = NULL;
@@ -1225,6 +1226,7 @@ exit_loop:
 fail:
 av_dict_free();
 ff_thread_report_progress(>picture, INT_MAX, 0);
+ff_thread_report_progress(>previous_picture, INT_MAX, 0);
 return ret;
 }
 
-- 
1.7.9.5

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