Re: [FFmpeg-devel] [PATCH] Check av_dup_packet() return code

2016-06-05 Thread Michael Niedermayer
On Thu, Mar 03, 2016 at 05:15:07PM +0100, Michael Niedermayer wrote:
> On Wed, Feb 03, 2016 at 11:27:00PM +0100, Andreas Cadhalpun wrote:
> > On 03.02.2016 19:05, Michael Niedermayer wrote:
> > > Fixes: CID1338320
> > > 
> > > Signed-off-by: Michael Niedermayer 
> > > ---
> > >  libavcodec/frame_thread_encoder.c |4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/libavcodec/frame_thread_encoder.c 
> > > b/libavcodec/frame_thread_encoder.c
> > > index 04c9a0e..27ae356 100644
> > > --- a/libavcodec/frame_thread_encoder.c
> > > +++ b/libavcodec/frame_thread_encoder.c
> > > @@ -89,7 +89,9 @@ static void * attribute_align_arg worker(void *v){
> > >  pthread_mutex_unlock(>buffer_mutex);
> > >  av_frame_free();
> > >  if(got_packet) {
> > > -av_dup_packet(pkt);
> > > +int ret2 = av_dup_packet(pkt);
> > > +if (ret >= 0 && ret2 < 0)
> > > +ret = ret2;
> > >  } else {
> > >  pkt->data = NULL;
> > >  pkt->size = 0;
> > > 
> > 
> > Checking the return code this way is probably OK, but maybe you can also 
> > replace the
> > deprecated av_dup_packet with av_packet_ref while at it?
> 
> ill apply this patch in a few days if there are no objections
> as noone suggested another solution

now what was the definition of few 

forgotten
applied

thx

[...]


-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Breaking DRM is a little like attempting to break through a door even
though the window is wide open and the only thing in the house is a bunch
of things you dont want and which you would get tomorrow for free anyway


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


Re: [FFmpeg-devel] [PATCH] Check av_dup_packet() return code

2016-03-03 Thread Michael Niedermayer
On Wed, Feb 03, 2016 at 11:27:00PM +0100, Andreas Cadhalpun wrote:
> On 03.02.2016 19:05, Michael Niedermayer wrote:
> > Fixes: CID1338320
> > 
> > Signed-off-by: Michael Niedermayer 
> > ---
> >  libavcodec/frame_thread_encoder.c |4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/libavcodec/frame_thread_encoder.c 
> > b/libavcodec/frame_thread_encoder.c
> > index 04c9a0e..27ae356 100644
> > --- a/libavcodec/frame_thread_encoder.c
> > +++ b/libavcodec/frame_thread_encoder.c
> > @@ -89,7 +89,9 @@ static void * attribute_align_arg worker(void *v){
> >  pthread_mutex_unlock(>buffer_mutex);
> >  av_frame_free();
> >  if(got_packet) {
> > -av_dup_packet(pkt);
> > +int ret2 = av_dup_packet(pkt);
> > +if (ret >= 0 && ret2 < 0)
> > +ret = ret2;
> >  } else {
> >  pkt->data = NULL;
> >  pkt->size = 0;
> > 
> 
> Checking the return code this way is probably OK, but maybe you can also 
> replace the
> deprecated av_dup_packet with av_packet_ref while at it?

ill apply this patch in a few days if there are no objections
as noone suggested another solution

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

it is not once nor twice but times without number that the same ideas make
their appearance in the world. -- Aristotle


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


Re: [FFmpeg-devel] [PATCH] Check av_dup_packet() return code

2016-02-08 Thread Michael Niedermayer
On Mon, Feb 08, 2016 at 12:46:11PM +0100, wm4 wrote:
> On Mon, 8 Feb 2016 00:33:15 +0100
> Michael Niedermayer  wrote:
> 
> > On Wed, Feb 03, 2016 at 11:27:00PM +0100, Andreas Cadhalpun wrote:
> > > On 03.02.2016 19:05, Michael Niedermayer wrote:  
> > > > Fixes: CID1338320
> > > > 
> > > > Signed-off-by: Michael Niedermayer 
> > > > ---
> > > >  libavcodec/frame_thread_encoder.c |4 +++-
> > > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/libavcodec/frame_thread_encoder.c 
> > > > b/libavcodec/frame_thread_encoder.c
> > > > index 04c9a0e..27ae356 100644
> > > > --- a/libavcodec/frame_thread_encoder.c
> > > > +++ b/libavcodec/frame_thread_encoder.c
> > > > @@ -89,7 +89,9 @@ static void * attribute_align_arg worker(void *v){
> > > >  pthread_mutex_unlock(>buffer_mutex);
> > > >  av_frame_free();
> > > >  if(got_packet) {
> > > > -av_dup_packet(pkt);
> > > > +int ret2 = av_dup_packet(pkt);
> > > > +if (ret >= 0 && ret2 < 0)
> > > > +ret = ret2;
> > > >  } else {
> > > >  pkt->data = NULL;
> > > >  pkt->size = 0;
> > > >   
> > > 
> > > Checking the return code this way is probably OK, but maybe you can also 
> > > replace the
> > > deprecated av_dup_packet with av_packet_ref while at it?  
> > 
> > what exactly do you suggest ?
> > 
> > av_packet_ref() creates a reference, av_dup_packet() turns "reusable"
> > packets into ref counted ones
> > av_dup_packet() does nothing if the packet is already a ref counted
> > one.
> > 
> > i can not see a efficient and clean way to replace av_dup_packet()
> > by av_packet_ref()
> > 
> > if its ref counted already nothing needs to be done, if its not
> > ref counted i would have to litterally reimplement av_dup_packet()
> > and i dont even have a testcase for that, all cases seem to return
> > ref counted packets
> > 
> > i can remove av_dup_packet and put an assert there assuming that
> > all packets will always be already ref counted (this might be true
> > currently, i dont know, i also dont know if we always want this to
> > be true in the future)
> > 
> > I dont really want to always run av_packet_ref() over it just to
> > unref it in the next line, which probably would work too
> > 
> > [...]
> 
> It's deprecated?

av_dup_packet() has been marked as deprecated, yes, the function or a
equivalent is needed though for efficicent support of non ref counted
packets.
Because what it does is exactly that it ensures that the packet
after the call is ref counted

iam happy with any solution the community likes

av_dup_packet() can be undeprecated or aded back under a different name

support for non reference counted packets can be removed (with the
obvious consequence that its then not supported anymore)

a redundant reference can always be added (this feels silly though as
it performs operations which take cpu time& thread sync when they are
entirely unneeded)

something else, there likely is some other option ...


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

Those who would give up essential Liberty, to purchase a little
temporary Safety, deserve neither Liberty nor Safety -- Benjamin Franklin


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


Re: [FFmpeg-devel] [PATCH] Check av_dup_packet() return code

2016-02-08 Thread wm4
On Mon, 8 Feb 2016 00:33:15 +0100
Michael Niedermayer  wrote:

> On Wed, Feb 03, 2016 at 11:27:00PM +0100, Andreas Cadhalpun wrote:
> > On 03.02.2016 19:05, Michael Niedermayer wrote:  
> > > Fixes: CID1338320
> > > 
> > > Signed-off-by: Michael Niedermayer 
> > > ---
> > >  libavcodec/frame_thread_encoder.c |4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/libavcodec/frame_thread_encoder.c 
> > > b/libavcodec/frame_thread_encoder.c
> > > index 04c9a0e..27ae356 100644
> > > --- a/libavcodec/frame_thread_encoder.c
> > > +++ b/libavcodec/frame_thread_encoder.c
> > > @@ -89,7 +89,9 @@ static void * attribute_align_arg worker(void *v){
> > >  pthread_mutex_unlock(>buffer_mutex);
> > >  av_frame_free();
> > >  if(got_packet) {
> > > -av_dup_packet(pkt);
> > > +int ret2 = av_dup_packet(pkt);
> > > +if (ret >= 0 && ret2 < 0)
> > > +ret = ret2;
> > >  } else {
> > >  pkt->data = NULL;
> > >  pkt->size = 0;
> > >   
> > 
> > Checking the return code this way is probably OK, but maybe you can also 
> > replace the
> > deprecated av_dup_packet with av_packet_ref while at it?  
> 
> what exactly do you suggest ?
> 
> av_packet_ref() creates a reference, av_dup_packet() turns "reusable"
> packets into ref counted ones
> av_dup_packet() does nothing if the packet is already a ref counted
> one.
> 
> i can not see a efficient and clean way to replace av_dup_packet()
> by av_packet_ref()
> 
> if its ref counted already nothing needs to be done, if its not
> ref counted i would have to litterally reimplement av_dup_packet()
> and i dont even have a testcase for that, all cases seem to return
> ref counted packets
> 
> i can remove av_dup_packet and put an assert there assuming that
> all packets will always be already ref counted (this might be true
> currently, i dont know, i also dont know if we always want this to
> be true in the future)
> 
> I dont really want to always run av_packet_ref() over it just to
> unref it in the next line, which probably would work too
> 
> [...]

It's deprecated?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] Check av_dup_packet() return code

2016-02-07 Thread Michael Niedermayer
On Wed, Feb 03, 2016 at 11:27:00PM +0100, Andreas Cadhalpun wrote:
> On 03.02.2016 19:05, Michael Niedermayer wrote:
> > Fixes: CID1338320
> > 
> > Signed-off-by: Michael Niedermayer 
> > ---
> >  libavcodec/frame_thread_encoder.c |4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/libavcodec/frame_thread_encoder.c 
> > b/libavcodec/frame_thread_encoder.c
> > index 04c9a0e..27ae356 100644
> > --- a/libavcodec/frame_thread_encoder.c
> > +++ b/libavcodec/frame_thread_encoder.c
> > @@ -89,7 +89,9 @@ static void * attribute_align_arg worker(void *v){
> >  pthread_mutex_unlock(>buffer_mutex);
> >  av_frame_free();
> >  if(got_packet) {
> > -av_dup_packet(pkt);
> > +int ret2 = av_dup_packet(pkt);
> > +if (ret >= 0 && ret2 < 0)
> > +ret = ret2;
> >  } else {
> >  pkt->data = NULL;
> >  pkt->size = 0;
> > 
> 
> Checking the return code this way is probably OK, but maybe you can also 
> replace the
> deprecated av_dup_packet with av_packet_ref while at it?

what exactly do you suggest ?

av_packet_ref() creates a reference, av_dup_packet() turns "reusable"
packets into ref counted ones
av_dup_packet() does nothing if the packet is already a ref counted
one.

i can not see a efficient and clean way to replace av_dup_packet()
by av_packet_ref()

if its ref counted already nothing needs to be done, if its not
ref counted i would have to litterally reimplement av_dup_packet()
and i dont even have a testcase for that, all cases seem to return
ref counted packets

i can remove av_dup_packet and put an assert there assuming that
all packets will always be already ref counted (this might be true
currently, i dont know, i also dont know if we always want this to
be true in the future)

I dont really want to always run av_packet_ref() over it just to
unref it in the next line, which probably would work too

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

Dictatorship: All citizens are under surveillance, all their steps and
actions recorded, for the politicians to enforce control.
Democracy: All politicians are under surveillance, all their steps and
actions recorded, for the citizens to enforce control.


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


Re: [FFmpeg-devel] [PATCH] Check av_dup_packet() return code

2016-02-03 Thread Andreas Cadhalpun
On 03.02.2016 19:05, Michael Niedermayer wrote:
> Fixes: CID1338320
> 
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/frame_thread_encoder.c |4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/frame_thread_encoder.c 
> b/libavcodec/frame_thread_encoder.c
> index 04c9a0e..27ae356 100644
> --- a/libavcodec/frame_thread_encoder.c
> +++ b/libavcodec/frame_thread_encoder.c
> @@ -89,7 +89,9 @@ static void * attribute_align_arg worker(void *v){
>  pthread_mutex_unlock(>buffer_mutex);
>  av_frame_free();
>  if(got_packet) {
> -av_dup_packet(pkt);
> +int ret2 = av_dup_packet(pkt);
> +if (ret >= 0 && ret2 < 0)
> +ret = ret2;
>  } else {
>  pkt->data = NULL;
>  pkt->size = 0;
> 

Checking the return code this way is probably OK, but maybe you can also 
replace the
deprecated av_dup_packet with av_packet_ref while at it?

Best regards,
Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH] Check av_dup_packet() return code

2016-02-03 Thread Michael Niedermayer
Fixes: CID1338320

Signed-off-by: Michael Niedermayer 
---
 libavcodec/frame_thread_encoder.c |4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/libavcodec/frame_thread_encoder.c 
b/libavcodec/frame_thread_encoder.c
index 04c9a0e..27ae356 100644
--- a/libavcodec/frame_thread_encoder.c
+++ b/libavcodec/frame_thread_encoder.c
@@ -89,7 +89,9 @@ static void * attribute_align_arg worker(void *v){
 pthread_mutex_unlock(>buffer_mutex);
 av_frame_free();
 if(got_packet) {
-av_dup_packet(pkt);
+int ret2 = av_dup_packet(pkt);
+if (ret >= 0 && ret2 < 0)
+ret = ret2;
 } else {
 pkt->data = NULL;
 pkt->size = 0;
-- 
1.7.9.5

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