diff mbox series

[FFmpeg-devel,v2,1/2] avcodec/decode: use a single list bsf for codec decode bsfs

Message ID 20200426083445.17883-1-cus@passwd.hu
State Accepted
Headers show
Series [FFmpeg-devel,v2,1/2] avcodec/decode: use a single list bsf for codec decode bsfs | expand

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Marton Balint April 26, 2020, 8:34 a.m. UTC 
Signed-off-by: Marton Balint <cus@passwd.hu>
---
 libavcodec/cuviddec.c |   2 +-
 libavcodec/decode.c   | 162 +++++++-------------------------------------------
 libavcodec/internal.h |   3 +-
 3 files changed, 23 insertions(+), 144 deletions(-)

Comments

James Almer April 26, 2020, 10:51 p.m. UTC | #1 
On 4/26/2020 5:34 AM, Marton Balint wrote:
>  void avcodec_flush_buffers(AVCodecContext *avctx)
>  {
>      AVCodecInternal *avci = avctx->internal;
> @@ -2117,7 +2001,7 @@ void avcodec_flush_buffers(AVCodecContext *avctx)
>      avctx->pts_correction_last_pts =
>      avctx->pts_correction_last_dts = INT64_MIN;
>  
> -    bsfs_flush(avctx);
> +    av_bsf_flush(avci->filter.bsf);

This function can be called with encoders as well, and after this change
you'll be calling av_bsf_flush() with NULL as argument.

Easiest solution is to add an av_codec_is_decoder(avctx->codec) check
before calling it, i guess.

>  
>      if (!avctx->refcounted_frames)
>          av_frame_unref(avci->to_free);
Marton Balint April 28, 2020, 5:57 p.m. UTC | #2 
On Sun, 26 Apr 2020, James Almer wrote:

> On 4/26/2020 5:34 AM, Marton Balint wrote:
>>  void avcodec_flush_buffers(AVCodecContext *avctx)
>>  {
>>      AVCodecInternal *avci = avctx->internal;
>> @@ -2117,7 +2001,7 @@ void avcodec_flush_buffers(AVCodecContext *avctx)
>>      avctx->pts_correction_last_pts =
>>      avctx->pts_correction_last_dts = INT64_MIN;
>> 
>> -    bsfs_flush(avctx);
>> +    av_bsf_flush(avci->filter.bsf);
>
> This function can be called with encoders as well, and after this change
> you'll be calling av_bsf_flush() with NULL as argument.
>
> Easiest solution is to add an av_codec_is_decoder(avctx->codec) check
> before calling it, i guess.

Ok, changed locally.

Regards,
Marton
Marton Balint May 1, 2020, 3:41 p.m. UTC | #3 
On Tue, 28 Apr 2020, Marton Balint wrote:

>
>
> On Sun, 26 Apr 2020, James Almer wrote:
>
>> On 4/26/2020 5:34 AM, Marton Balint wrote:
>>>  void avcodec_flush_buffers(AVCodecContext *avctx)
>>>  {
>>>      AVCodecInternal *avci = avctx->internal;
>>> @@ -2117,7 +2001,7 @@ void avcodec_flush_buffers(AVCodecContext *avctx)
>>>      avctx->pts_correction_last_pts =
>>>      avctx->pts_correction_last_dts = INT64_MIN;
>>> 
>>> -    bsfs_flush(avctx);
>>> +    av_bsf_flush(avci->filter.bsf);
>>
>> This function can be called with encoders as well, and after this change
>> you'll be calling av_bsf_flush() with NULL as argument.
>>
>> Easiest solution is to add an av_codec_is_decoder(avctx->codec) check
>> before calling it, i guess.
>
> Ok, changed locally.

Will apply the series soon, I will probably squash the two patch, since 
nobody was against removing DecodeFilterContext.

Regards,
Marton
Marton Balint May 2, 2020, 5:40 p.m. UTC | #4 
On Fri, 1 May 2020, Marton Balint wrote:

>
>
> On Tue, 28 Apr 2020, Marton Balint wrote:
>
>>
>>
>> On Sun, 26 Apr 2020, James Almer wrote:
>>
>>> On 4/26/2020 5:34 AM, Marton Balint wrote:
>>>>  void avcodec_flush_buffers(AVCodecContext *avctx)
>>>>  {
>>>>      AVCodecInternal *avci = avctx->internal;
>>>> @@ -2117,7 +2001,7 @@ void avcodec_flush_buffers(AVCodecContext *avctx)
>>>>      avctx->pts_correction_last_pts =
>>>>      avctx->pts_correction_last_dts = INT64_MIN;
>>>> 
>>>> -    bsfs_flush(avctx);
>>>> +    av_bsf_flush(avci->filter.bsf);
>>>
>>> This function can be called with encoders as well, and after this change
>>> you'll be calling av_bsf_flush() with NULL as argument.
>>>
>>> Easiest solution is to add an av_codec_is_decoder(avctx->codec) check
>>> before calling it, i guess.
>>
>> Ok, changed locally.
>
> Will apply the series soon, I will probably squash the two patch, since 
> nobody was against removing DecodeFilterContext.

Applied.

Regards,
Marton
diff mbox series

Patch

diff --git a/libavcodec/cuviddec.c b/libavcodec/cuviddec.c
index 50dc8956c3..13a7db10cd 100644
--- a/libavcodec/cuviddec.c
+++ b/libavcodec/cuviddec.c
@@ -946,7 +946,7 @@  static av_cold int cuvid_decode_init(AVCodecContext *avctx)
     }
 
     if (avctx->codec->bsfs) {
-        const AVCodecParameters *par = avctx->internal->filter.bsfs[avctx->internal->filter.nb_bsfs - 1]->par_out;
+        const AVCodecParameters *par = avctx->internal->filter.bsf->par_out;
         ctx->cuparse_ext.format.seqhdr_data_length = par->extradata_size;
         memcpy(ctx->cuparse_ext.raw_seqhdr_data,
                par->extradata,
diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index d4bdb9b1c0..576efd0e49 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -205,100 +205,30 @@  int ff_decode_bsfs_init(AVCodecContext *avctx)
 {
     AVCodecInternal *avci = avctx->internal;
     DecodeFilterContext *s = &avci->filter;
-    const char *bsfs_str;
     int ret;
 
-    if (s->nb_bsfs)
+    if (s->bsf)
         return 0;
 
-    bsfs_str = avctx->codec->bsfs ? avctx->codec->bsfs : "null";
-    while (bsfs_str && *bsfs_str) {
-        AVBSFContext **tmp;
-        const AVBitStreamFilter *filter;
-        char *bsf, *bsf_options_str, *bsf_name;
-
-        bsf = av_get_token(&bsfs_str, ",");
-        if (!bsf) {
-            ret = AVERROR(ENOMEM);
-            goto fail;
-        }
-        bsf_name = av_strtok(bsf, "=", &bsf_options_str);
-        if (!bsf_name) {
-            av_freep(&bsf);
-            ret = AVERROR(ENOMEM);
-            goto fail;
-        }
-
-        filter = av_bsf_get_by_name(bsf_name);
-        if (!filter) {
-            av_log(avctx, AV_LOG_ERROR, "A non-existing bitstream filter %s "
-                   "requested by a decoder. This is a bug, please report it.\n",
-                   bsf_name);
-            av_freep(&bsf);
+    ret = av_bsf_list_parse_str(avctx->codec->bsfs, &s->bsf);
+    if (ret < 0) {
+        av_log(avctx, AV_LOG_ERROR, "Error parsing decoder bitstream filters '%s': %s\n", avctx->codec->bsfs, av_err2str(ret));
+        if (ret != AVERROR(ENOMEM))
             ret = AVERROR_BUG;
-            goto fail;
-        }
-
-        tmp = av_realloc_array(s->bsfs, s->nb_bsfs + 1, sizeof(*s->bsfs));
-        if (!tmp) {
-            av_freep(&bsf);
-            ret = AVERROR(ENOMEM);
-            goto fail;
-        }
-        s->bsfs = tmp;
-
-        ret = av_bsf_alloc(filter, &s->bsfs[s->nb_bsfs]);
-        if (ret < 0) {
-            av_freep(&bsf);
-            goto fail;
-        }
-        s->nb_bsfs++;
-
-        if (s->nb_bsfs == 1) {
-            /* We do not currently have an API for passing the input timebase into decoders,
-             * but no filters used here should actually need it.
-             * So we make up some plausible-looking number (the MPEG 90kHz timebase) */
-            s->bsfs[s->nb_bsfs - 1]->time_base_in = (AVRational){ 1, 90000 };
-            ret = avcodec_parameters_from_context(s->bsfs[s->nb_bsfs - 1]->par_in,
-                                                  avctx);
-        } else {
-            s->bsfs[s->nb_bsfs - 1]->time_base_in = s->bsfs[s->nb_bsfs - 2]->time_base_out;
-            ret = avcodec_parameters_copy(s->bsfs[s->nb_bsfs - 1]->par_in,
-                                          s->bsfs[s->nb_bsfs - 2]->par_out);
-        }
-        if (ret < 0) {
-            av_freep(&bsf);
-            goto fail;
-        }
-
-        if (bsf_options_str && filter->priv_class) {
-            const AVOption *opt = av_opt_next(s->bsfs[s->nb_bsfs - 1]->priv_data, NULL);
-            const char * shorthand[2] = {NULL};
-
-            if (opt)
-                shorthand[0] = opt->name;
-
-            ret = av_opt_set_from_string(s->bsfs[s->nb_bsfs - 1]->priv_data, bsf_options_str, shorthand, "=", ":");
-            if (ret < 0) {
-                if (ret != AVERROR(ENOMEM)) {
-                    av_log(avctx, AV_LOG_ERROR, "Invalid options for bitstream filter %s "
-                           "requested by the decoder. This is a bug, please report it.\n",
-                           bsf_name);
-                    ret = AVERROR_BUG;
-                }
-                av_freep(&bsf);
-                goto fail;
-            }
-        }
-        av_freep(&bsf);
+        goto fail;
+    }
 
-        ret = av_bsf_init(s->bsfs[s->nb_bsfs - 1]);
-        if (ret < 0)
-            goto fail;
+    /* We do not currently have an API for passing the input timebase into decoders,
+     * but no filters used here should actually need it.
+     * So we make up some plausible-looking number (the MPEG 90kHz timebase) */
+    s->bsf->time_base_in = (AVRational){ 1, 90000 };
+    ret = avcodec_parameters_from_context(s->bsf->par_in, avctx);
+    if (ret < 0)
+        goto fail;
 
-        if (*bsfs_str)
-            bsfs_str++;
-    }
+    ret = av_bsf_init(s->bsf);
+    if (ret < 0)
+        goto fail;
 
     return 0;
 fail:
@@ -306,44 +236,6 @@  fail:
     return ret;
 }
 
-/* try to get one output packet from the filter chain */
-static int bsfs_poll(AVCodecContext *avctx, AVPacket *pkt)
-{
-    DecodeFilterContext *s = &avctx->internal->filter;
-    int idx, ret;
-
-    /* start with the last filter in the chain */
-    idx = s->nb_bsfs - 1;
-    while (idx >= 0) {
-        /* request a packet from the currently selected filter */
-        ret = av_bsf_receive_packet(s->bsfs[idx], pkt);
-        if (ret == AVERROR(EAGAIN)) {
-            /* no packets available, try the next filter up the chain */
-            idx--;
-            continue;
-        } else if (ret < 0 && ret != AVERROR_EOF) {
-            return ret;
-        }
-
-        /* got a packet or EOF -- pass it to the caller or to the next filter
-         * down the chain */
-        if (idx == s->nb_bsfs - 1) {
-            return ret;
-        } else {
-            idx++;
-            ret = av_bsf_send_packet(s->bsfs[idx], ret < 0 ? NULL : pkt);
-            if (ret < 0) {
-                av_log(avctx, AV_LOG_ERROR,
-                       "Error pre-processing a packet before decoding\n");
-                av_packet_unref(pkt);
-                return ret;
-            }
-        }
-    }
-
-    return AVERROR(EAGAIN);
-}
-
 int ff_decode_get_packet(AVCodecContext *avctx, AVPacket *pkt)
 {
     AVCodecInternal *avci = avctx->internal;
@@ -352,7 +244,7 @@  int ff_decode_get_packet(AVCodecContext *avctx, AVPacket *pkt)
     if (avci->draining)
         return AVERROR_EOF;
 
-    ret = bsfs_poll(avctx, pkt);
+    ret = av_bsf_receive_packet(avci->filter.bsf, pkt);
     if (ret == AVERROR_EOF)
         avci->draining = 1;
     if (ret < 0)
@@ -713,7 +605,7 @@  int attribute_align_arg avcodec_send_packet(AVCodecContext *avctx, const AVPacke
             return ret;
     }
 
-    ret = av_bsf_send_packet(avci->filter.bsfs[0], avci->buffer_pkt);
+    ret = av_bsf_send_packet(avci->filter.bsf, avci->buffer_pkt);
     if (ret < 0) {
         av_packet_unref(avci->buffer_pkt);
         return ret;
@@ -2072,14 +1964,6 @@  int ff_reget_buffer(AVCodecContext *avctx, AVFrame *frame, int flags)
     return ret;
 }
 
-static void bsfs_flush(AVCodecContext *avctx)
-{
-    DecodeFilterContext *s = &avctx->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;
@@ -2117,7 +2001,7 @@  void avcodec_flush_buffers(AVCodecContext *avctx)
     avctx->pts_correction_last_pts =
     avctx->pts_correction_last_dts = INT64_MIN;
 
-    bsfs_flush(avctx);
+    av_bsf_flush(avci->filter.bsf);
 
     if (!avctx->refcounted_frames)
         av_frame_unref(avci->to_free);
@@ -2126,10 +2010,6 @@  void avcodec_flush_buffers(AVCodecContext *avctx)
 void ff_decode_bsfs_uninit(AVCodecContext *avctx)
 {
     DecodeFilterContext *s = &avctx->internal->filter;
-    int i;
 
-    for (i = 0; i < s->nb_bsfs; i++)
-        av_bsf_free(&s->bsfs[i]);
-    av_freep(&s->bsfs);
-    s->nb_bsfs = 0;
+    av_bsf_free(&s->bsf);
 }
diff --git a/libavcodec/internal.h b/libavcodec/internal.h
index 721fd017d4..35a15c9664 100644
--- a/libavcodec/internal.h
+++ b/libavcodec/internal.h
@@ -114,8 +114,7 @@  typedef struct DecodeSimpleContext {
 } DecodeSimpleContext;
 
 typedef struct DecodeFilterContext {
-    AVBSFContext **bsfs;
-    int         nb_bsfs;
+    AVBSFContext *bsf;
 } DecodeFilterContext;
 
 typedef struct AVCodecInternal {