Re: [FFmpeg-devel] [PATCH 4/7] lavc: move decoder bsf init into decoder-specific code

2021-03-10 Thread James Almer 

On 3/10/2021 10:52 AM, Andreas Rheinhardt wrote:

James Almer:

On 3/10/2021 9:03 AM, Anton Khirnov wrote:

---
   libavcodec/decode.c | 8 +++-
   libavcodec/decode.h | 6 --
   libavcodec/utils.c  | 6 --
   3 files changed, 7 insertions(+), 13 deletions(-)

diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index e5a301ec58..d25b15e95a 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -184,7 +184,7 @@ static int extract_packet_props(AVCodecInternal
*avci, const AVPacket *pkt)
   return 0;
   }
   -int ff_decode_bsfs_init(AVCodecContext *avctx)
+static int decode_bsfs_init(AVCodecContext *avctx)
   {
   AVCodecInternal *avci = avctx->internal;
   int ret;
@@ -2010,6 +2010,8 @@ int ff_reget_buffer(AVCodecContext *avctx,
AVFrame *frame, int flags)
     int ff_decode_preinit(AVCodecContext *avctx)
   {
+    int ret = 0;
+
   /* if the decoder init function was already called previously,
    * free the already allocated subtitle_header before overwriting
it */
   av_freep(>subtitle_header);
@@ -2046,5 +2048,9 @@ FF_ENABLE_DEPRECATION_WARNINGS
   avctx->export_side_data |= AV_CODEC_EXPORT_DATA_MVS;
   }
   +    ret = decode_bsfs_init(avctx);


Did you try to run fate with THREADS set to something other than 1? This
moves the bsfs initialization after ff_thread_init(), so all the copies
of avctx in frame threading scenarios will be missing the bsfs requested
by the decoder.



For frame threaded decoders the codec's decode function is directly run
with the child AVCodecContext; they don't need the bsfs: The parent's
bsf has already been run on the packets before the frame threaded code
is reached.


Alright, should be ok, then.




fate-vp9 should be enough to confirm this, since afaik it's the only
frame threaded decoder that uses a bsf.


+    if (ret < 0)
+    return ret;
+
   return 0;
   }
diff --git a/libavcodec/decode.h b/libavcodec/decode.h
index a865fe954f..1467b1eb33 100644
--- a/libavcodec/decode.h
+++ b/libavcodec/decode.h
@@ -69,12 +69,6 @@ int ff_decode_get_packet(AVCodecContext *avctx,
AVPacket *pkt);
    */
   int ff_decode_frame_props(AVCodecContext *avctx, AVFrame *frame);
   -/**
- * Called during avcodec_open2() to initialize avctx->internal->bsf.
- * The bsf should be freed with av_bsf_free().
- */
-int ff_decode_bsfs_init(AVCodecContext *avctx);
-
   /**
    * Make sure avctx.hw_frames_ctx is set. If it's not set, the
function will
    * try to allocate it from hw_device_ctx. If that is not possible,
an error
diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index 6d5aeb4eaf..918cb1b4d1 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -730,12 +730,6 @@ int attribute_align_arg
avcodec_open2(AVCodecContext *avctx, const AVCodec *code
   goto free_and_end;
   }
   -    if (av_codec_is_decoder(avctx->codec)) {
-    ret = ff_decode_bsfs_init(avctx);
-    if (ret < 0)
-    goto free_and_end;
-    }
-
   if (HAVE_THREADS
   && !(avci->frame_thread_encoder &&
(avctx->active_thread_type_THREAD_FRAME))) {
   ret = ff_thread_init(avctx);



___
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 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 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 4/7] lavc: move decoder bsf init into decoder-specific code

2021-03-10 Thread Andreas Rheinhardt 
James Almer:
> On 3/10/2021 9:03 AM, Anton Khirnov wrote:
>> ---
>>   libavcodec/decode.c | 8 +++-
>>   libavcodec/decode.h | 6 --
>>   libavcodec/utils.c  | 6 --
>>   3 files changed, 7 insertions(+), 13 deletions(-)
>>
>> diff --git a/libavcodec/decode.c b/libavcodec/decode.c
>> index e5a301ec58..d25b15e95a 100644
>> --- a/libavcodec/decode.c
>> +++ b/libavcodec/decode.c
>> @@ -184,7 +184,7 @@ static int extract_packet_props(AVCodecInternal
>> *avci, const AVPacket *pkt)
>>   return 0;
>>   }
>>   -int ff_decode_bsfs_init(AVCodecContext *avctx)
>> +static int decode_bsfs_init(AVCodecContext *avctx)
>>   {
>>   AVCodecInternal *avci = avctx->internal;
>>   int ret;
>> @@ -2010,6 +2010,8 @@ int ff_reget_buffer(AVCodecContext *avctx,
>> AVFrame *frame, int flags)
>>     int ff_decode_preinit(AVCodecContext *avctx)
>>   {
>> +    int ret = 0;
>> +
>>   /* if the decoder init function was already called previously,
>>    * free the already allocated subtitle_header before overwriting
>> it */
>>   av_freep(>subtitle_header);
>> @@ -2046,5 +2048,9 @@ FF_ENABLE_DEPRECATION_WARNINGS
>>   avctx->export_side_data |= AV_CODEC_EXPORT_DATA_MVS;
>>   }
>>   +    ret = decode_bsfs_init(avctx);
> 
> Did you try to run fate with THREADS set to something other than 1? This
> moves the bsfs initialization after ff_thread_init(), so all the copies
> of avctx in frame threading scenarios will be missing the bsfs requested
> by the decoder.
> 

For frame threaded decoders the codec's decode function is directly run
with the child AVCodecContext; they don't need the bsfs: The parent's
bsf has already been run on the packets before the frame threaded code
is reached.

> fate-vp9 should be enough to confirm this, since afaik it's the only
> frame threaded decoder that uses a bsf.
> 
>> +    if (ret < 0)
>> +    return ret;
>> +
>>   return 0;
>>   }
>> diff --git a/libavcodec/decode.h b/libavcodec/decode.h
>> index a865fe954f..1467b1eb33 100644
>> --- a/libavcodec/decode.h
>> +++ b/libavcodec/decode.h
>> @@ -69,12 +69,6 @@ int ff_decode_get_packet(AVCodecContext *avctx,
>> AVPacket *pkt);
>>    */
>>   int ff_decode_frame_props(AVCodecContext *avctx, AVFrame *frame);
>>   -/**
>> - * Called during avcodec_open2() to initialize avctx->internal->bsf.
>> - * The bsf should be freed with av_bsf_free().
>> - */
>> -int ff_decode_bsfs_init(AVCodecContext *avctx);
>> -
>>   /**
>>    * Make sure avctx.hw_frames_ctx is set. If it's not set, the
>> function will
>>    * try to allocate it from hw_device_ctx. If that is not possible,
>> an error
>> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
>> index 6d5aeb4eaf..918cb1b4d1 100644
>> --- a/libavcodec/utils.c
>> +++ b/libavcodec/utils.c
>> @@ -730,12 +730,6 @@ int attribute_align_arg
>> avcodec_open2(AVCodecContext *avctx, const AVCodec *code
>>   goto free_and_end;
>>   }
>>   -    if (av_codec_is_decoder(avctx->codec)) {
>> -    ret = ff_decode_bsfs_init(avctx);
>> -    if (ret < 0)
>> -    goto free_and_end;
>> -    }
>> -
>>   if (HAVE_THREADS
>>   && !(avci->frame_thread_encoder &&
>> (avctx->active_thread_type_THREAD_FRAME))) {
>>   ret = ff_thread_init(avctx);
>>
> 
> ___
> 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 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 4/7] lavc: move decoder bsf init into decoder-specific code

2021-03-10 Thread James Almer 

On 3/10/2021 9:03 AM, Anton Khirnov wrote:

---
  libavcodec/decode.c | 8 +++-
  libavcodec/decode.h | 6 --
  libavcodec/utils.c  | 6 --
  3 files changed, 7 insertions(+), 13 deletions(-)

diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index e5a301ec58..d25b15e95a 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -184,7 +184,7 @@ static int extract_packet_props(AVCodecInternal *avci, 
const AVPacket *pkt)
  return 0;
  }
  
-int ff_decode_bsfs_init(AVCodecContext *avctx)

+static int decode_bsfs_init(AVCodecContext *avctx)
  {
  AVCodecInternal *avci = avctx->internal;
  int ret;
@@ -2010,6 +2010,8 @@ int ff_reget_buffer(AVCodecContext *avctx, AVFrame 
*frame, int flags)
  
  int ff_decode_preinit(AVCodecContext *avctx)

  {
+int ret = 0;
+
  /* if the decoder init function was already called previously,
   * free the already allocated subtitle_header before overwriting it */
  av_freep(>subtitle_header);
@@ -2046,5 +2048,9 @@ FF_ENABLE_DEPRECATION_WARNINGS
  avctx->export_side_data |= AV_CODEC_EXPORT_DATA_MVS;
  }
  
+ret = decode_bsfs_init(avctx);


Did you try to run fate with THREADS set to something other than 1? This 
moves the bsfs initialization after ff_thread_init(), so all the copies 
of avctx in frame threading scenarios will be missing the bsfs requested 
by the decoder.


fate-vp9 should be enough to confirm this, since afaik it's the only 
frame threaded decoder that uses a bsf.



+if (ret < 0)
+return ret;
+
  return 0;
  }
diff --git a/libavcodec/decode.h b/libavcodec/decode.h
index a865fe954f..1467b1eb33 100644
--- a/libavcodec/decode.h
+++ b/libavcodec/decode.h
@@ -69,12 +69,6 @@ int ff_decode_get_packet(AVCodecContext *avctx, AVPacket 
*pkt);
   */
  int ff_decode_frame_props(AVCodecContext *avctx, AVFrame *frame);
  
-/**

- * Called during avcodec_open2() to initialize avctx->internal->bsf.
- * The bsf should be freed with av_bsf_free().
- */
-int ff_decode_bsfs_init(AVCodecContext *avctx);
-
  /**
   * Make sure avctx.hw_frames_ctx is set. If it's not set, the function will
   * try to allocate it from hw_device_ctx. If that is not possible, an error
diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index 6d5aeb4eaf..918cb1b4d1 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -730,12 +730,6 @@ int attribute_align_arg avcodec_open2(AVCodecContext 
*avctx, const AVCodec *code
  goto free_and_end;
  }
  
-if (av_codec_is_decoder(avctx->codec)) {

-ret = ff_decode_bsfs_init(avctx);
-if (ret < 0)
-goto free_and_end;
-}
-
  if (HAVE_THREADS
  && !(avci->frame_thread_encoder && 
(avctx->active_thread_type_THREAD_FRAME))) {
  ret = ff_thread_init(avctx);



___
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 4/7] lavc: move decoder bsf init into decoder-specific code

2021-03-10 Thread Anton Khirnov 
---
 libavcodec/decode.c | 8 +++-
 libavcodec/decode.h | 6 --
 libavcodec/utils.c  | 6 --
 3 files changed, 7 insertions(+), 13 deletions(-)

diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index e5a301ec58..d25b15e95a 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -184,7 +184,7 @@ static int extract_packet_props(AVCodecInternal *avci, 
const AVPacket *pkt)
 return 0;
 }
 
-int ff_decode_bsfs_init(AVCodecContext *avctx)
+static int decode_bsfs_init(AVCodecContext *avctx)
 {
 AVCodecInternal *avci = avctx->internal;
 int ret;
@@ -2010,6 +2010,8 @@ int ff_reget_buffer(AVCodecContext *avctx, AVFrame 
*frame, int flags)
 
 int ff_decode_preinit(AVCodecContext *avctx)
 {
+int ret = 0;
+
 /* if the decoder init function was already called previously,
  * free the already allocated subtitle_header before overwriting it */
 av_freep(>subtitle_header);
@@ -2046,5 +2048,9 @@ FF_ENABLE_DEPRECATION_WARNINGS
 avctx->export_side_data |= AV_CODEC_EXPORT_DATA_MVS;
 }
 
+ret = decode_bsfs_init(avctx);
+if (ret < 0)
+return ret;
+
 return 0;
 }
diff --git a/libavcodec/decode.h b/libavcodec/decode.h
index a865fe954f..1467b1eb33 100644
--- a/libavcodec/decode.h
+++ b/libavcodec/decode.h
@@ -69,12 +69,6 @@ int ff_decode_get_packet(AVCodecContext *avctx, AVPacket 
*pkt);
  */
 int ff_decode_frame_props(AVCodecContext *avctx, AVFrame *frame);
 
-/**
- * Called during avcodec_open2() to initialize avctx->internal->bsf.
- * The bsf should be freed with av_bsf_free().
- */
-int ff_decode_bsfs_init(AVCodecContext *avctx);
-
 /**
  * Make sure avctx.hw_frames_ctx is set. If it's not set, the function will
  * try to allocate it from hw_device_ctx. If that is not possible, an error
diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index 6d5aeb4eaf..918cb1b4d1 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -730,12 +730,6 @@ int attribute_align_arg avcodec_open2(AVCodecContext 
*avctx, const AVCodec *code
 goto free_and_end;
 }
 
-if (av_codec_is_decoder(avctx->codec)) {
-ret = ff_decode_bsfs_init(avctx);
-if (ret < 0)
-goto free_and_end;
-}
-
 if (HAVE_THREADS
 && !(avci->frame_thread_encoder && 
(avctx->active_thread_type_THREAD_FRAME))) {
 ret = ff_thread_init(avctx);
-- 
2.30.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".