Re: [FFmpeg-devel] [PATCH 1/4] avcodec: move avcodec_flush_buffers from decode.c to utils.c

2020-04-16 Thread Philip Langdale
On Thu, 16 Apr 2020 14:07:49 -0300
James Almer  wrote:

> On 4/16/2020 1:26 PM, Philip Langdale wrote:
> > On Tue, 10 Mar 2020 16:33:41 -0300
> > James Almer  wrote:  
> >>
> >> I can withdraw this patch (And remove the relevant chunks from the
> >> following two) if this function was effectively not meant for
> >> encoders. Also maybe poke Timo regarding nvenc flushing, to see why
> >> it is needed and if there's an alternative.  
> > 
> > I've pushed the change to formalise the encoder flush semantics, so
> > this should be good to apply now.  
> 
> All encoders were ported, so yes, it could be pushed. But i wonder if
> this should go in after 4.3 is tagged rather than now. I'd like this
> to have some months of testing in master before making it into a
> release.
> 
> Unless 4.3 is months away, in which case i'll just push it now.

Ok. You'll have a better sense of 4.3 timing than me, so I'll defer to
your judgement.

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/4] avcodec: move avcodec_flush_buffers from decode.c to utils.c

2020-04-16 Thread James Almer
On 4/16/2020 1:26 PM, Philip Langdale wrote:
> On Tue, 10 Mar 2020 16:33:41 -0300
> James Almer  wrote:
>>
>> I can withdraw this patch (And remove the relevant chunks from the
>> following two) if this function was effectively not meant for
>> encoders. Also maybe poke Timo regarding nvenc flushing, to see why
>> it is needed and if there's an alternative.
> 
> I've pushed the change to formalise the encoder flush semantics, so
> this should be good to apply now.

All encoders were ported, so yes, it could be pushed. But i wonder if
this should go in after 4.3 is tagged rather than now. I'd like this to
have some months of testing in master before making it into a release.

Unless 4.3 is months away, in which case i'll just push it now.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/4] avcodec: move avcodec_flush_buffers from decode.c to utils.c

2020-04-16 Thread Philip Langdale
On Tue, 10 Mar 2020 16:33:41 -0300
James Almer  wrote:
> 
> I can withdraw this patch (And remove the relevant chunks from the
> following two) if this function was effectively not meant for
> encoders. Also maybe poke Timo regarding nvenc flushing, to see why
> it is needed and if there's an alternative.

I've pushed the change to formalise the encoder flush semantics, so
this should be good to apply now.

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/4] avcodec: move avcodec_flush_buffers from decode.c to utils.c

2020-03-10 Thread James Almer
On 3/10/2020 4:08 PM, Anton Khirnov wrote:
> Quoting James Almer (2020-03-10 19:14:47)
>> On 3/10/2020 3:03 PM, Anton Khirnov wrote:
>>> Quoting James Almer (2020-02-27 19:01:59)
 It's not a decoding exclusive function.
>>>
>>> How come? The doxy for it says
>>> 'Reset the internal decoder state / flush internal buffers.'
>>
>> It touches some fields used by encode.c, like draining and
>> buffer_pkt_valid (the latter which admittedly will be gone after this
>> patchset, but replaced by others),
> 
> draining is for decoders too I think
> 
>> and there's at least one encoder implementing an AVCodec.flush()
>> callback.
>>
>> And doesn't the slash in that doxy imply one or the other?
> 
> Hmm, apparently I wrote that doxy and have no recollection of it :)
> I think it was supposed to be decoding only.

I made the question because i interpreted it as such, and seeing
encoders adding flush callbacks, the same might have done others.
It wasn't the correct interpretation, so good to know.

> 
> I see two encoders implementing flush: audiotoolbox and nvenc. The
> addition to nvenc was explicit, but not even a year old, audiotoolbox
> might or might not have been deliberate.
> 
> So it seems pretty unclear to me. I would say calling it on an encoder
> is invalid and undefined, but apparently people are doing that and it
> sort of sometimes works.

I can withdraw this patch (And remove the relevant chunks from the
following two) if this function was effectively not meant for encoders.
Also maybe poke Timo regarding nvenc flushing, to see why it is needed
and if there's an alternative.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/4] avcodec: move avcodec_flush_buffers from decode.c to utils.c

2020-03-10 Thread Anton Khirnov
Quoting James Almer (2020-03-10 19:14:47)
> On 3/10/2020 3:03 PM, Anton Khirnov wrote:
> > Quoting James Almer (2020-02-27 19:01:59)
> >> It's not a decoding exclusive function.
> > 
> > How come? The doxy for it says
> > 'Reset the internal decoder state / flush internal buffers.'
> 
> It touches some fields used by encode.c, like draining and
> buffer_pkt_valid (the latter which admittedly will be gone after this
> patchset, but replaced by others),

draining is for decoders too I think

> and there's at least one encoder implementing an AVCodec.flush()
> callback.
> 
> And doesn't the slash in that doxy imply one or the other?

Hmm, apparently I wrote that doxy and have no recollection of it :)
I think it was supposed to be decoding only.

I see two encoders implementing flush: audiotoolbox and nvenc. The
addition to nvenc was explicit, but not even a year old, audiotoolbox
might or might not have been deliberate.

So it seems pretty unclear to me. I would say calling it on an encoder
is invalid and undefined, but apparently people are doing that and it
sort of sometimes works.

-- 
Anton Khirnov
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/4] avcodec: move avcodec_flush_buffers from decode.c to utils.c

2020-03-10 Thread James Almer
On 3/10/2020 3:03 PM, Anton Khirnov wrote:
> Quoting James Almer (2020-02-27 19:01:59)
>> It's not a decoding exclusive function.
> 
> How come? The doxy for it says
> 'Reset the internal decoder state / flush internal buffers.'

It touches some fields used by encode.c, like draining and
buffer_pkt_valid (the latter which admittedly will be gone after this
patchset, but replaced by others), and there's at least one encoder
implementing an AVCodec.flush() callback.

And doesn't the slash in that doxy imply one or the other?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/4] avcodec: move avcodec_flush_buffers from decode.c to utils.c

2020-03-10 Thread Anton Khirnov
Quoting James Almer (2020-02-27 19:01:59)
> It's not a decoding exclusive function.

How come? The doxy for it says
'Reset the internal decoder state / flush internal buffers.'

-- 
Anton Khirnov
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 1/4] avcodec: move avcodec_flush_buffers from decode.c to utils.c

2020-02-27 Thread James Almer
It's not a decoding exclusive function.

Signed-off-by: James Almer 
---
 libavcodec/decode.c | 36 
 libavcodec/utils.c  | 33 +
 2 files changed, 33 insertions(+), 36 deletions(-)

diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index 03b9da25f9..93b0db7ef8 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -2020,42 +2020,6 @@ int ff_reget_buffer(AVCodecContext *avctx, AVFrame 
*frame, int flags)
 return ret;
 }
 
-static void bsfs_flush(AVCodecContext *avctx)
-{
-DecodeFilterContext *s = >internal->filter;
-
-for (int i = 0; i < s->nb_bsfs; i++)
-av_bsf_flush(s->bsfs[i]);
-}
-
-void avcodec_flush_buffers(AVCodecContext *avctx)
-{
-AVCodecInternal *avci = avctx->internal;
-
-avci->draining  = 0;
-avci->draining_done = 0;
-avci->nb_draining_errors = 0;
-av_frame_unref(avci->buffer_frame);
-av_frame_unref(avci->compat_decode_frame);
-av_packet_unref(avci->buffer_pkt);
-avci->buffer_pkt_valid = 0;
-
-av_packet_unref(avci->ds.in_pkt);
-
-if (HAVE_THREADS && avctx->active_thread_type & FF_THREAD_FRAME)
-ff_thread_flush(avctx);
-else if (avctx->codec->flush)
-avctx->codec->flush(avctx);
-
-avctx->pts_correction_last_pts =
-avctx->pts_correction_last_dts = INT64_MIN;
-
-bsfs_flush(avctx);
-
-if (!avctx->refcounted_frames)
-av_frame_unref(avci->to_free);
-}
-
 void ff_decode_bsfs_uninit(AVCodecContext *avctx)
 {
 DecodeFilterContext *s = >internal->filter;
diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index c4dc136d3c..5257a1c627 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -1084,6 +1084,39 @@ FF_ENABLE_DEPRECATION_WARNINGS
 goto end;
 }
 
+void avcodec_flush_buffers(AVCodecContext *avctx)
+{
+AVCodecInternal *avci = avctx->internal;
+
+avci->draining  = 0;
+avci->draining_done = 0;
+avci->nb_draining_errors = 0;
+av_frame_unref(avci->buffer_frame);
+av_frame_unref(avci->compat_decode_frame);
+av_packet_unref(avci->buffer_pkt);
+avci->buffer_pkt_valid = 0;
+
+av_packet_unref(avci->ds.in_pkt);
+
+if (HAVE_THREADS && avctx->active_thread_type & FF_THREAD_FRAME)
+ff_thread_flush(avctx);
+else if (avctx->codec->flush)
+avctx->codec->flush(avctx);
+
+avctx->pts_correction_last_pts =
+avctx->pts_correction_last_dts = INT64_MIN;
+
+if (av_codec_is_decoder(avctx->codec)) {
+DecodeFilterContext *s = >internal->filter;
+
+for (int i = 0; i < s->nb_bsfs; i++)
+av_bsf_flush(s->bsfs[i]);
+}
+
+if (!avctx->refcounted_frames)
+av_frame_unref(avci->to_free);
+}
+
 void avsubtitle_free(AVSubtitle *sub)
 {
 int i;
-- 
2.25.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-requ...@ffmpeg.org with subject "unsubscribe".