Re: [FFmpeg-devel] [PATCH] Check av_dup_packet() return code
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
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
On Mon, Feb 08, 2016 at 12:46:11PM +0100, wm4 wrote: > On Mon, 8 Feb 2016 00:33:15 +0100 > Michael Niedermayerwrote: > > > 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
On Mon, 8 Feb 2016 00:33:15 +0100 Michael Niedermayerwrote: > 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
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
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
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