Re: [FFmpeg-devel] [PATCH 03/24] avcodec/subtitles: Introduce new frame-based subtitle decoding API

[Andreas Rheinhardt <andreas.rheinhardt@outlook.com>]

ffmpegagent:
> From: softworkz <softworkz@hotmail.com>
> 
> - Add avcodec_decode_subtitle3 which takes subtitle frames,
>   serving as compatibility shim to legacy subtitle decoding
> - Add additional methods for conversion between old and new API

This commit message is completely wrong.

> 
> Signed-off-by: softworkz <softworkz@hotmail.com>
> ---
>  libavcodec/avcodec.h    |   8 +-
>  libavcodec/codec_desc.c |  11 +++
>  libavcodec/codec_desc.h |   8 ++
>  libavcodec/decode.c     |  56 ++++++++++--
>  libavcodec/internal.h   |  22 +++++
>  libavcodec/utils.c      | 184 ++++++++++++++++++++++++++++++++++++++++
>  6 files changed, 280 insertions(+), 9 deletions(-)
> 
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index fe5a83cf85..9d59f6e840 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -1675,7 +1675,7 @@ typedef struct AVCodecContext {
>  
>      /**
>       * Header containing style information for text subtitles.
> -     * For SUBTITLE_ASS subtitle type, it should contain the whole ASS
> +     * For AV_SUBTITLE_FMT_ASS subtitle type, it should contain the whole ASS
>       * [Script Info] and [V4+ Styles] section, plus the [Events] line and
>       * the Format line following. It shouldn't include any Dialogue line.
>       * - encoding: Set/allocated/freed by user (before avcodec_open2())
> @@ -2415,7 +2415,10 @@ int avcodec_close(AVCodecContext *avctx);
>   * Free all allocated data in the given subtitle struct.
>   *
>   * @param sub AVSubtitle to free.
> + *
> + * @deprecated Use the regular frame based encode and decode APIs instead.
>   */
> +attribute_deprecated
>  void avsubtitle_free(AVSubtitle *sub);
>  
>  /**
> @@ -2508,7 +2511,10 @@ enum AVChromaLocation avcodec_chroma_pos_to_enum(int xpos, int ypos);
>   *                 must be freed with avsubtitle_free if *got_sub_ptr is set.
>   * @param[in,out] got_sub_ptr Zero if no subtitle could be decompressed, otherwise, it is nonzero.
>   * @param[in] avpkt The input AVPacket containing the input buffer.
> + *
> + * @deprecated Use the new decode API (avcodec_send_packet, avcodec_receive_frame) instead.
>   */
> +attribute_deprecated
>  int avcodec_decode_subtitle2(AVCodecContext *avctx, AVSubtitle *sub,
>                              int *got_sub_ptr,
>                              AVPacket *avpkt);
> diff --git a/libavcodec/codec_desc.c b/libavcodec/codec_desc.c
> index 0974ee03de..e48e4532ba 100644
> --- a/libavcodec/codec_desc.c
> +++ b/libavcodec/codec_desc.c
> @@ -3548,3 +3548,14 @@ enum AVMediaType avcodec_get_type(enum AVCodecID codec_id)
>      const AVCodecDescriptor *desc = avcodec_descriptor_get(codec_id);
>      return desc ? desc->type : AVMEDIA_TYPE_UNKNOWN;
>  }
> +
> +enum AVSubtitleType avcodec_descriptor_get_subtitle_format(const AVCodecDescriptor *codec_descriptor)
> +{
> +    if(codec_descriptor->props & AV_CODEC_PROP_BITMAP_SUB)
> +        return AV_SUBTITLE_FMT_BITMAP;
> +
> +    if(codec_descriptor->props & AV_CODEC_PROP_TEXT_SUB)
> +        return AV_SUBTITLE_FMT_ASS;
> +
> +    return AV_SUBTITLE_FMT_UNKNOWN;
> +}
> diff --git a/libavcodec/codec_desc.h b/libavcodec/codec_desc.h
> index 126b52df47..ba68d24e0e 100644
> --- a/libavcodec/codec_desc.h
> +++ b/libavcodec/codec_desc.h
> @@ -121,6 +121,14 @@ const AVCodecDescriptor *avcodec_descriptor_next(const AVCodecDescriptor *prev);
>   */
>  const AVCodecDescriptor *avcodec_descriptor_get_by_name(const char *name);
>  
> +/**
> + * Return subtitle format from a codec descriptor
> + *
> + * @param codec_descriptor codec descriptor
> + * @return                 the subtitle type (e.g. bitmap, text)
> + */
> +enum AVSubtitleType avcodec_descriptor_get_subtitle_format(const AVCodecDescriptor *codec_descriptor);
> +
>  /**
>   * @}
>   */
> diff --git a/libavcodec/decode.c b/libavcodec/decode.c
> index 0912f86a14..ab8a6ea6ff 100644
> --- a/libavcodec/decode.c
> +++ b/libavcodec/decode.c
> @@ -576,6 +576,39 @@ static int decode_receive_frame_internal(AVCodecContext *avctx, AVFrame *frame)
>      return ret;
>  }
>  
> +static int decode_subtitle2_priv(AVCodecContext *avctx, AVSubtitle *sub,
> +                                 int *got_sub_ptr, AVPacket *avpkt);
> +
> +static int decode_subtitle_shim(AVCodecContext *avctx, AVFrame *frame, AVPacket *avpkt)
> +{
> +    int ret, got_sub_ptr = 0;
> +    AVSubtitle subtitle = { 0 };
> +
> +    if (frame->buf[0])
> +        return AVERROR(EAGAIN);
> +
> +    av_frame_unref(frame);
> +
> +    ret = decode_subtitle2_priv(avctx, &subtitle, &got_sub_ptr, avpkt);
> +
> +    if (ret >= 0 && got_sub_ptr) {
> +        frame->type = AVMEDIA_TYPE_SUBTITLE;
> +        frame->format = subtitle.format;
> +        ret = av_frame_get_buffer2(frame, 0);
> +
> +        if (ret >= 0)
> +            ret = ff_frame_put_subtitle(frame, &subtitle);
> +
> +        frame->width = avctx->width;
> +        frame->height = avctx->height;
> +        frame->pkt_dts = avpkt->dts;
> +    }
> +
> +    avsubtitle_free(&subtitle);
> +
> +    return ret;
> +}
> +
>  int attribute_align_arg avcodec_send_packet(AVCodecContext *avctx, const AVPacket *avpkt)
>  {
>      AVCodecInternal *avci = avctx->internal;
> @@ -590,6 +623,9 @@ int attribute_align_arg avcodec_send_packet(AVCodecContext *avctx, const AVPacke
>      if (avpkt && !avpkt->size && avpkt->data)
>          return AVERROR(EINVAL);
>  
> +    if (avctx->codec_type == AVMEDIA_TYPE_SUBTITLE)
> +        return decode_subtitle_shim(avctx, avci->buffer_frame, avpkt);

With the avcodec_send_packet/avcodec_receive_frame API the user is
supposed to send the packet only once (unless we have the EAGAIN case).
Libavcodec takes care to buffer the packet in case this is necessary
even if the packet leads to multiple returned frames. In contrast to
this, the old API required the user to send the packet again, but with
data offsetted and size decremented to reflect this.
Some decoders (at least pgssubdec*) seem to be designed to only consume
partial packets; if that is the case, then your approach above won't
work and you will need to buffer the packet. In other words: The
subtitle decoding codepath will become more like the codepaths for audio
and video.
The old API is btw even more broken, in particular in case the subtitles
are recoded: Typically the size of the recoded UTF-8 is returned which
is an API abuse on part of lavc. See
https://github.com/mkver/FFmpeg/commit/ba1564c532654888015d67b70bf73d117c2d9f75
(part of https://github.com/mkver/FFmpeg/commits/subs) for more.
*: It seems that libzvbi currently always fully consumes the AVPacket;
yet looking at the history I don't know whether this is actually
intended and correct.

> +
>      av_packet_unref(avci->buffer_pkt);
>      if (avpkt && (avpkt->data || avpkt->side_data_elems)) {
>          ret = av_packet_ref(avci->buffer_pkt, avpkt);
> @@ -651,7 +687,9 @@ int attribute_align_arg avcodec_receive_frame(AVCodecContext *avctx, AVFrame *fr
>  
>      if (avci->buffer_frame->buf[0]) {
>          av_frame_move_ref(frame, avci->buffer_frame);
> -    } else {
> +    } else if (avctx->codec_type == AVMEDIA_TYPE_SUBTITLE)
> +        return AVERROR(EAGAIN);
> +    else {
>          ret = decode_receive_frame_internal(avctx, frame);
>          if (ret < 0)
>              return ret;
> @@ -802,9 +840,8 @@ static int utf8_check(const uint8_t *str)
>      return 1;
>  }
>  
> -int avcodec_decode_subtitle2(AVCodecContext *avctx, AVSubtitle *sub,
> -                             int *got_sub_ptr,
> -                             AVPacket *avpkt)
> +static int decode_subtitle2_priv(AVCodecContext *avctx, AVSubtitle *sub,
> +                             int *got_sub_ptr, AVPacket *avpkt)
>  {
>      int ret = 0;
>  
> @@ -850,10 +887,7 @@ int avcodec_decode_subtitle2(AVCodecContext *avctx, AVSubtitle *sub,
>                                                   avctx->pkt_timebase, ms);
>          }
>  
> -        if (avctx->codec_descriptor->props & AV_CODEC_PROP_BITMAP_SUB)
> -            sub->format = 0;
> -        else if (avctx->codec_descriptor->props & AV_CODEC_PROP_TEXT_SUB)
> -            sub->format = 1;
> +        sub->format = avcodec_descriptor_get_subtitle_format(avctx->codec_descriptor);
>  
>          for (unsigned i = 0; i < sub->num_rects; i++) {
>              if (avctx->sub_charenc_mode != FF_SUB_CHARENC_MODE_IGNORE &&
> @@ -874,6 +908,12 @@ int avcodec_decode_subtitle2(AVCodecContext *avctx, AVSubtitle *sub,
>      return ret;
>  }
>  
> +int avcodec_decode_subtitle2(AVCodecContext *avctx, AVSubtitle *sub,
> +                             int *got_sub_ptr, AVPacket *avpkt)
> +{
> +    return decode_subtitle2_priv(avctx, sub, got_sub_ptr, avpkt);
> +}
> +
>  enum AVPixelFormat avcodec_default_get_format(struct AVCodecContext *avctx,
>                                                const enum AVPixelFormat *fmt)
>  {
> diff --git a/libavcodec/internal.h b/libavcodec/internal.h
> index 72ca1553f6..ff63974f7f 100644
> --- a/libavcodec/internal.h
> +++ b/libavcodec/internal.h
> @@ -363,4 +363,26 @@ int ff_int_from_list_or_default(void *ctx, const char * val_name, int val,
>  
>  void ff_dvdsub_parse_palette(uint32_t *palette, const char *p);
>  
> +/**
> + * Copies subtitle data from AVSubtitle to AVFrame.
> + *
> + * @deprecated This is a compatibility method for interoperability with
> + * the legacy subtitle API.
> + */
> +int ff_frame_put_subtitle(AVFrame* frame, const AVSubtitle* sub);
> +
> +/**
> + * Copies subtitle data from AVFrame to AVSubtitle.
> + *
> + * @deprecated This is a compatibility method for interoperability with
> + * the legacy subtitle API.
> + */
> +int ff_frame_get_subtitle(AVSubtitle* sub, AVFrame* frame);
> +
> +#if defined(_WIN32) && CONFIG_SHARED && !defined(BUILDING_avcodec)
> +#    define av_export_avcodec __declspec(dllimport)
> +#else
> +#    define av_export_avcodec
> +#endif

This seems like a rebase problem.

> +
>  #endif /* AVCODEC_INTERNAL_H */

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

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